Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'd love to improve this code. I know I should put a lot of stuff in another class but I have no idea how. Thanks for any kind of help. Any kind of tips are also welcome.

import java.util.Scanner;

public class UserInteraction {

    public static void main(String[] args) {

        Scanner scan = new Scanner(System.in);
        int choice = 0;
        String[] subjects = new String[10];
        int grades[] = new int[10];
        double sum = 0.0;

    do
    {
        System.out.println("1. Enter a course name and a grade");
        System.out.println("2. Display all grades");
        System.out.println("3. Calculate the average grade");
        System.out.println("4. Exit program");

        choice = scan.nextInt();

        if ( choice == 1 ) 
        {
            Scanner scansubjects = new Scanner(System.in);
            Scanner scangrades = new Scanner(System.in);

            System.out.println("Enter 10 subjects and their corresponding grades:");
            System.out.println();

            int i = 0;

            for( i = 0; i < 10; i++ )
            {
                System.out.println("Subject:");

                String temp = scansubjects.nextLine();
                subjects[i] = temp.toLowerCase();

                System.out.println("Grade:");

                grades[i] = scangrades.nextInt();

                if( i == ( subjects.length - 1 ) )
                {
                    System.out.println("Thank you!");
                    System.out.println();
                }
            }
        }


        if ( choice == 2 )
        {
            System.out.println("Subjects" + "\tGrades");
            System.out.println("---------------------");

            for(int p = 0; p < subjects.length; p++)
            {

                System.out.println(subjects[p] + "\t" + "\t" + grades[p]);
            }
        }

        if ( choice == 3 )
        {   
              System.out.println("Total of grades: " + getSum(grades));
              System.out.println("Count of grades: " + grades.length);
              System.out.println("Average of grades: " + getAverage(grades));
              System.out.println();
        }


    } while ( choice != 4);


}

    public static double getAverage(int[] array)
    {
        int sum = 0;
        for(int i : array) sum += i;
        return ((double) sum)/array.length;
    }

    public static double getSum(int[] array)
    {
        int sum = 0;
        for (int i : array) 
        {
         sum += i;
        }
        return sum;
    }

}
share|improve this question

1 Answer

up vote 1 down vote accepted

Simple refactored example:

public class UserInteraction {

    public static void main(String[] args) {
        UserChoiceManager userChoiceManager = new UserChoiceManager();
        UserChoice choice = UserChoice.UNKNOWN;

        do {
            choice = userChoiceManager.manage();
        } while (choice != UserChoice.EXIT);

        System.out.println("Bye!");
    }
}

public enum UserChoice {
    UNKNOWN(0),
    ENTER_SUBJECT(1),
    DISPLAY_DATA(2),
    CALCULATE_AVERAGE_GRADE(3),
    EXIT(4);

    private int id;

    public int getId() {
        return id;
    }

    UserChoice(int id) {
        this.id = id;
    }

    public static UserChoice getById(int input) {
        for (UserChoice value : UserChoice.values()) {
            if (value.getId() == input) {
                return value;
            }
        }
        return UserChoice.UNKNOWN;
    }

}

public class Subject {

    private String name;

    private int grade;

    public Subject(String name, int grade) {
        this.name = name;
        this.grade = grade;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getGrade() {
        return grade;
    }

    public void setGrade(int grade) {
        this.grade = grade;
    }

}

public class UserChoiceManager {

    private final static int SUBJECT_COUNT = 10;

    private Subject[] subjects = new Subject[SUBJECT_COUNT];

    private Scanner scan = new Scanner(System.in);

    public UserChoiceManager() {
    }

    public UserChoice manage() {
        System.out.println(printHelp());

        UserChoice choice = getUserChoiceById(scan);

        if (choice == UserChoice.ENTER_SUBJECT) {
            choice = enterSubjects(choice);
        }

        if (choice == UserChoice.DISPLAY_DATA) {
            printSubjectsAndGrades();
        }

        if (choice == UserChoice.CALCULATE_AVERAGE_GRADE) {
            printAverageGrade();
        }

        return choice;
    }

    private static String printHelp() {
        StringBuilder usage = new StringBuilder("\nUsage:\n");
        usage.append(UserChoice.ENTER_SUBJECT.getId()).append(". Enter a course name and a grade").append("\n");
        usage.append(UserChoice.DISPLAY_DATA.getId()).append(". Display all grades").append("\n");
        usage.append(UserChoice.CALCULATE_AVERAGE_GRADE.getId()).append(". Calculate the average grade").append("\n");
        usage.append(UserChoice.EXIT.getId()).append(". Exit program");

        return usage.toString();

    }

    private UserChoice getUserChoiceById(Scanner scan) {
        return UserChoice.getById(scan.nextInt());
    }

    private UserChoice enterSubjects(UserChoice choice) {
        System.out.println("Enter " + subjects.length + " subjects and their corresponding grades:");
        System.out.println();

        for (int i = 0; i < subjects.length; i++) {
            String subjectName = scanSubjectName();
            int grade = scanGrade();
            subjects[i] = new Subject(subjectName.toLowerCase(), grade);
        }

        System.out.println("Thank you!");
        System.out.println();

        return choice;
    }

    private String scanSubjectName() {
        Scanner temp = new Scanner(System.in);
        String subjectName = "";
        do {
            System.out.println("Subject:");
            subjectName = temp.nextLine();
            if (subjectName.equals("")) {
                System.out.println("Empty subject name! Try again.");
            }
        } while (subjectName.equals(""));

        return subjectName;
    }

    private int scanGrade() {
        int grade = 0;
        do {
            Scanner temp = new Scanner(System.in);
            System.out.println("Grade:");
            try {
                grade = temp.nextInt();
            } catch (InputMismatchException e) {
                System.out.println("Invalid grade. Enter numeric value! Try again.");
            }
        } while (grade == 0);

        return grade;
    }

    private void printAverageGrade() {
        System.out.println("Total of grades: " + getSum(subjects));
        System.out.println("Count of grades: " + subjects.length);
        System.out.println("Average of grades: " + getAverage(subjects));
        System.out.println();
    }

    private void printSubjectsAndGrades() {
        System.out.println("Subjects" + "\tGrades");
        System.out.println("---------------------");

        for (int p = 0; p < subjects.length; p++) {
            System.out.println(subjects[p].getName() + "\t" + "\t" + subjects[p].getGrade());
        }
    }

    public static double getAverage(Subject[] subjects) {
        int sum = 0;
        for (Subject s : subjects)
            sum += s.getGrade();
        return ((double) sum) / subjects.length;
    }

    public static double getSum(Subject[] subjects) {
        int sum = 0;
        for (Subject s : subjects) {
            sum += s.getGrade();
        }
        return sum;

    }
}
share|improve this answer
Thank you for taking your time. This is very helpful. – videojames Nov 4 '12 at 13:49
May I suggest using a switch statement instead of ifs in the manage method. What happens if you get the UNKOWN enum value, at the moment you don't seem to handle it in any way? Also in the getAverage method, you have repeated all the logic already in getSum - just call getSum rather than repeating the loop. – luketorjussen Nov 4 '12 at 14:36

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.