2
\$\begingroup\$

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.

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;
    }

}
\$\endgroup\$
3
\$\begingroup\$

Let's go through refactoring your code step by step.

Get (almost) everything out of main()

main() should consist of the fewest lines to launch the program. In this case you only need one.

    new UserInteraction().run();

Or, for clarity, you can break that into two lines.

    UserInteraction ui = new UserInteraction();
    ui.run();

You would then put everything from main() into a method called run(). The actually name of the new method is not important. You can use start() if you like.

This step may seem almost silly at first, but it is very important. First, all your methods and variables stop needing to be static. For a beginner, you should avoid static as much as possible. Second, it starts you thinking in an object oriented way (as opposed to procedurally). Here's the first consequence of getting everything out of main():

    private double getAverage(int[] array)
    ...
    private double getSum(int[] array)

Notice that the methods are no longer static. This is a good thing. I also changed the accessibility to private. Like static, a member should be private unless there is a good reason for it not to be.

Methods should do one thing well

So now we have a monolithic run() method doing all sorts of this. Let's make run() only do one thing well. We're shooting for something like this:

    private void run() {
        int choice = 0;

        do {
            showMenu();
            choice = scan.nextInt();

            switch (choice) {
            case 1:
                enterCourses();
                break;
            case 2:
                displayGrades();
                break;
            case 3:
                displayAverageGrade();
            }
        } while (choice != 4);
    }

Now run() only concerns itself with one thing: running the basic flow of the program.

Declare variables in the tightest scope possible

So where are all the variable declarations? In general, you want to declare your variables "as close as you can." Variable scope is a little tricky but for a beginner, try to declare your variables in the same block as their used. (A block is roughly the code between braces {}). choice needs to be declared where it is because it's used outside the switch block.

You only need one scanner object and it needs to be declared once. It will be used in several methods, so it needs to be declared outside of all methods but inside of a class.

    public class UserInteraction {
        Scanner scan = new Scanner(System.in);

        public static void main(String[] args) { // ...

Let's leave the rest of the variable declarations and initializations for later and move code into our new methods.

    private void showMenu() {
        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");
    }

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

        for (int i = 0; i < subjects.length; i++) {
            System.out.println("Subject:");
            subjects[i] = scan.nextLine();

            System.out.println("Grade:");
            grades[i] = scan.nextInt();
            scan.nextLine();

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

    private void displayGrades() {
        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]);
        }
    }

    private void displayAverageGrade() {
        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();
    }

Now you can see that subjects and grades need to have class scope, that is, to be declared outside of all methods. And we notice that declaring sum in run() is unnecessary because a local variable (one inside a method) will do.

Scanner weirdness

The scanner class has some oddities that I can't go into at length, but the long and the short is that you either put the line scan.nextLine() after each scan.nextInt(), or write a wrapper around scanner to hide the weirdness, or, like you did, have two scanner objects, one for strings and one for ints. The wrapper is best, but as you can imagine, it's the hardest. The nice thing though is if you write it correctly, you can reuse it anywhere.

Final thoughts

There is a lot more you can do, but I need to wrap this up. Here are some things to think about.

  • You could put all your display into one class
  • What happens when you enter a non-integer into the menu or grades prompts?
  • There's a better way to store the subjects and grades. What about a class that holds one subject and grade? How does that change the program?
  • You have repeated code in your getAverage() and getSum() methods. How can you get rid of that?

Okay, here's the "final" code, but not really final as there's a lot still to do:

import java.util.Scanner;

public class UserInteraction {
    private Scanner scan = new Scanner(System.in);
    private String[] subjects = new String[10];
    private int grades[] = new int[10];

    public static void main(String[] args) {
        new UserInteraction().run();
    }

    private void run() {
        int choice;

        do {
            showMenu();
            choice = scan.nextInt();
            scan.nextLine();

            switch (choice) {
            case 1:
                enterCourses();
                break;
            case 2:
                displayGrades();
                break;
            case 3:
                displayAverageGrade();
            }
        } while (choice != 4);
    }

    private void showMenu() {
        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");
    }

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

        for (int i = 0; i < subjects.length; i++) {
            System.out.println("Subject:");
            subjects[i] = scan.nextLine();

            System.out.println("Grade:");
            grades[i] = scan.nextInt();
            scan.nextLine();

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

    private void displayGrades() {
        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]);
        }
    }

    private void displayAverageGrade() {
        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();
    }

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

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

}   
\$\endgroup\$

protected by Jamal Sep 11 '17 at 0:41

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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