4
\$\begingroup\$

The program is complicated. I didn't call any class because it was hard for me using classes.

Did I do something redundant or did I miss something?

import java.util.Scanner;

public class Gpa {
    public static void main(String[] args){
        int i;
        Scanner keyboard = new Scanner(System.in);

        //7th term lessons
        String lessonOne[] ={"Electronics","Communication 2","Dsp","Robotics","Selection 1","Selection 2","Selection 3"};

        //8th term lessons
        String lessonTwo[] ={"DSD","Communication 1","Cmos","Biomedical","Feedback","Project"};

        //7th term credits
        float creditsOne[] = {4,4,3,3,3,3,3};

        //8th term credits
        float creditsTwo[] = {4,4,3,3,3,3};

        //total credits i obtained to date
        double total = 235;

        //prompts Lessons
        String myStr;
            System.out.println("Enter your 7th term notes:");
                   for(i=0; i<7; i++){
                    System.out.print(lessonOne[i]+":");

                myStr = keyboard.nextLine();

        // gives warning if these inputs are not entered
                while(!myStr.equals("AA") && !myStr.equals("aa")&& 
                      !myStr.equals("4.0") && !myStr.equals("4,0")&& 
                      !myStr.equals("4") && !myStr.equals("BA")&&
                      !myStr.equals("ba") && !myStr.equals("AB")&&
                      !myStr.equals("ab") && !myStr.equals("3.5")&&
                      !myStr.equals("3,5") && !myStr.equals("BB")&&
                      !myStr.equals("bb") && !myStr.equals("3,0")&&
                      !myStr.equals("3.0") && !myStr.equals("3")&&
                      !myStr.equals("CB") && !myStr.equals("cb")&&
                      !myStr.equals("BC") && !myStr.equals("bc")&&
                      !myStr.equals("2,5") && !myStr.equals("2.5")&&
                      !myStr.equals("CC")  && !myStr.equals("cc")&&
                      !myStr.equals("2.0") && !myStr.equals("2,0")&&
                      !myStr.equals("2") && !myStr.equals("DC")&&
                      !myStr.equals("CD") && !myStr.equals("dc")&&
                      !myStr.equals("cd")  &&!myStr.equals("1.5")&&
                      !myStr.equals("1,5") && !myStr.equals("DD")&&
                      !myStr.equals("dd") && !myStr.equals("1.0")&&
                      !myStr.equals("1,0") && !myStr.equals("1")&&
                      !myStr.equals("FD") && !myStr.equals("fd")&&
                      !myStr.equals("df") &&!myStr.equals("DF")&&
                      !myStr.equals("0.5") && !myStr.equals("0,5")&&
                      !myStr.equals("FF") &&!myStr.equals("ff")&&
                      !myStr.equals("0,0") && !myStr.equals("0.0")&&
                      !myStr.equals("0")
                      ) {
                            System.out.printf("Please enter another input:for example -> AA, aa, 4.0, 4,0 or 4");
                            myStr = keyboard.nextLine(); 
                }

                if(myStr.equals("AA")|| myStr.equals("aa")|| myStr.equals("4.0") || myStr.equals("4")|| myStr.equals("4,0"))
                    total = total + 4*creditsOne[i];

                else if(myStr.equals("BA")|| myStr.equals("ba")||myStr.equals("AB")|| myStr.equals("ab")|| myStr.equals("3.5") || myStr.equals("3,5"))
                    total = total + 3.5*creditsOne[i];

                else if(myStr.equals("BB")|| myStr.equals("bb")|| myStr.equals("3.0") || myStr.equals("3")|| myStr.equals("3,0"))
                    total = total + 3*creditsOne[i];

                else if(myStr.equals("CB")|| myStr.equals("cb")||myStr.equals("BC")|| myStr.equals("bc")|| myStr.equals("2.5") || myStr.equals("2,5"))
                    total = total + 2.5*creditsOne[i];

                else if(myStr.equals("CC")|| myStr.equals("cc")|| myStr.equals("2.0") || myStr.equals("2")|| myStr.equals("2,0"))
                    total = total + 2*creditsOne[i];

                else if(myStr.equals("DC")|| myStr.equals("dc")||myStr.equals("CD")|| myStr.equals("cd")|| myStr.equals("1.5") || myStr.equals("1,5"))
                    total = total + 1.5*creditsOne[i];

                else if(myStr.equals("DD")|| myStr.equals("dd")|| myStr.equals("1.0") || myStr.equals("1")|| myStr.equals("1,0"))
                    total = total + 1*creditsOne[i];

                else if(myStr.equals("FD")|| myStr.equals("fd")||myStr.equals("DF")|| myStr.equals("df")|| myStr.equals("0.5") || myStr.equals("0,5"))
                    total = total + 0.5*creditsOne[i];
                else if(myStr.equals("FF")|| myStr.equals("ff")|| myStr.equals("0,0") || myStr.equals("0.0")|| myStr.equals("0"))
                    total = total + 0*creditsOne[i];

    }
           System.out.printf("Your gpa for all 7 term: %.3f", (float)total/ 139);

           String myStr2;
           System.out.println("\nEnter  8th term notes:");
              for(i=0; i<6; i++){
              System.out.printf(lessonTwo[i]+":");

            myStr2 = keyboard.nextLine();

            // gives warning if these inputs are not entered
            while(!myStr2.equals("AA") && !myStr2.equals("aa")&& 
                  !myStr2.equals("4.0") && !myStr2.equals("4,0")&& 
                  !myStr2.equals("4") && !myStr2.equals("BA")&&
                  !myStr2.equals("ba") && !myStr2.equals("AB")&&
                  !myStr2.equals("ab") && !myStr2.equals("3.5")&&
                  !myStr2.equals("3,5") && !myStr2.equals("BB")&&
                  !myStr2.equals("bb") && !myStr2.equals("3,0")&&
                  !myStr2.equals("3.0") && !myStr2.equals("3")&&
                  !myStr2.equals("CB") && !myStr2.equals("cb")&&
                  !myStr2.equals("BC") && !myStr2.equals("bc")&&
                  !myStr2.equals("2,5") && !myStr2.equals("2.5")&&
                  !myStr2.equals("CC")  && !myStr2.equals("cc")&&
                  !myStr2.equals("2.0") && !myStr2.equals("2,0")&&
                  !myStr2.equals("2") && !myStr2.equals("DC")&&
                  !myStr2.equals("CD") && !myStr2.equals("dc")&&
                  !myStr2.equals("cd")  &&!myStr2.equals("1.5")&&
                  !myStr2.equals("1,5") && !myStr2.equals("DD")&&
                  !myStr2.equals("dd") && !myStr2.equals("1.0")&&
                  !myStr2.equals("1,0") && !myStr2.equals("1")&&
                  !myStr2.equals("FD") && !myStr2.equals("fd")&&
                  !myStr2.equals("df") &&!myStr2.equals("DF")&&
                  !myStr2.equals("0.5") && !myStr2.equals("0,5")&&
                  !myStr2.equals("FF") &&!myStr2.equals("ff")&&
                  !myStr2.equals("0,0") && !myStr2.equals("0.0")&&
                  !myStr2.equals("0")
                  ) {
                        System.out.printf("Please enter another input:for example -> AA, aa, 4.0, 4,0 or 4");
                        myStr2 = keyboard.nextLine(); 
            }

            if(myStr2.equals("AA")|| myStr2.equals("aa")|| myStr2.equals("4.0") || myStr2.equals("4")|| myStr2.equals("4,0"))
                total = total + 4*creditsOne[i];

            else if(myStr2.equals("BA")|| myStr2.equals("ba")||myStr2.equals("AB")|| myStr2.equals("ab")|| myStr2.equals("3.5") || myStr2.equals("3,5"))
                total = total + 3.5*creditsOne[i];

            else if(myStr2.equals("BB")|| myStr2.equals("bb")|| myStr2.equals("3.0") || myStr2.equals("3")|| myStr2.equals("3,0"))
                total = total + 3*creditsOne[i];

            else if(myStr2.equals("CB")|| myStr2.equals("cb")||myStr2.equals("BC")|| myStr2.equals("bc")|| myStr2.equals("2.5") || myStr2.equals("2,5"))
                total = total + 2.5*creditsOne[i];

            else if(myStr2.equals("CC")|| myStr2.equals("cc")|| myStr2.equals("2.0") || myStr2.equals("2")|| myStr2.equals("2,0"))
                total = total + 2*creditsOne[i];

            else if(myStr2.equals("DC")|| myStr2.equals("dc")||myStr2.equals("CD")|| myStr2.equals("cd")|| myStr2.equals("1.5") || myStr2.equals("1,5"))
                total = total + 1.5*creditsOne[i];

            else if(myStr2.equals("DD")|| myStr2.equals("dd")|| myStr2.equals("1.0") || myStr2.equals("1")|| myStr2.equals("1,0"))
                total = total + 1*creditsOne[i];

            else if(myStr2.equals("FD")|| myStr2.equals("fd")||myStr2.equals("DF")|| myStr2.equals("df")|| myStr2.equals("0.5") || myStr2.equals("0,5"))
                total = total + 0.5*creditsOne[i];
            else if(myStr2.equals("FF")|| myStr2.equals("ff")|| myStr2.equals("0,0") || myStr2.equals("0.0")|| myStr2.equals("0"))
                total = total + 0*creditsOne[i];



        }

            System.out.printf("Your final note %f",(float)total/142);

}



}
\$\endgroup\$
4
\$\begingroup\$

The most obvious issue that I noticed immediately with your code is that monstrous conditional in the while loop that you use to validate input. The primary concerns with it are:

  1. Difficult to maintain: By having to list every condition one by one use AND && it makes it both time-consuming and error-prone if/when you have to add or modify the criteria. For instance, if you forgot the NOT ! or accidentally used a single & instead of && could lead to obscure bugs. Doubly so since the whole conditional is repeated again later.

  2. You check for AA and aa, but what about aA or Aa? Neither of those would be considered as valid, even though they really should be. Instead of trying to handle every single possibility, just transform the input in a standard way so you only have to check for AA (or aa, your choice) by applying String.toUpperCase() (or toLowerCase()) to the input you are validating.

I would recommend to put all your valid inputs in a data structure, such as a List<String>, and then you can just check the input against that list. It could look like this, for example:

// this list contains all inputs considered as valid:
List<String> validInputs = Arrays.asList(
    "AA", "4.0", "4,0", "4",
    "BA", "AB", "3.5", "3,5",
    "BB", "3.0", "3,0", "3",
    // etc.
);

// gives warning if invalid inputs are entered:
while(!validInput.contains(myStr.toUpperCase()) {
    System.out.printf("Please enter another input:for example -> AA, aa, 4.0, 4,0 or 4");
    myStr = keyboard.nextLine(); 
}

It would also make sense to make a static method to handle this so you can simply call that method any time you need to validate input.



This set of if / else conditionals can be simplified with a switch construct:

if(myStr2.equals("AA")|| myStr2.equals("aa")|| myStr2.equals("4.0") || myStr2.equals("4")|| myStr2.equals("4,0"))
    total = total + 4*creditsOne[i];
else if(myStr2.equals("BA")|| myStr2.equals("ba")||myStr2.equals("AB")|| myStr2.equals("ab")|| myStr2.equals("3.5") || myStr2.equals("3,5"))
    total = total + 3.5*creditsOne[i];
// etc.

For example:

switch(myStr2.toUpperCase()) {
    case "AA":
    case "4.0":
    case "4,0":
    case "4":
        total += 4 * creditsOne[i];
        break;
    case "BA":
    case "AB":
    case "3.5":
    case "3,5":
        total += 3.5 * creditsOne[i];
        break;
    // etc.
    default:
        // handle default here, if applicable
}
\$\endgroup\$
3
\$\begingroup\$

Yes, there is a better way to do this. Let's identify the source of complexity first:

  • Long if-else statements

    !myStr.equals("AA") && !myStr.equals("aa") && !myStr.equals("4.0") && !myStr.equals("4,0")
    
  • Long conditions

    if (myStr.equals("AA") || myStr.equals("aa") || myStr.equals("4.0") || myStr.equals("4") || myStr.equals("4,0"))
        total = total + 4 * creditsOne[i];
    

Both of these can be improved by refactoring your code to use smaller functions:

// prompts Lessons
System.out.println("Enter your 7th term notes.");
for (i = 0; i < lessonOne.length; i++) { // we can use lessonOne.length instead of 7 here
    System.out.print(lessonOne[i] + ":");

    Grade grade = null;
    while (grade == null) {     
        // This code doesn't just appear after invalid input now, but you can change this easily
        System.out.printf("Please enter another input: for example -> AA, aa, 4.0, 4,0 or 4");
        grade = parseGrade(keyboard.nextLine());
    }   
}

To do this, we use the following helper code, which you can nest under your Gpa class:

private enum Grade {
    A(4.0), Ab(3.5), B(3.0), Bc(2.5), C(2.0), Cd(1.5), D(1.0), Df(0.5), F(0.0);

    private double value;

    private Grade(double value) {
        this.value = value;
    }

    public double toValue() {
        return value;
    }
}

// Returns null if missing (using java.util.Optional would be even better here)
private static Grade parseGrade(String grade) {
    grade = grade.toUpperCase(); // aa to AA

    // All of your complex parsing goes here, so that you don't have to repeat yourself

    if ("AA".equals(grade) || "4.0".equals(grade) || "4".equals(grade) || "4,0".equals(grade))
        return Grade.A;

    if ("AB".equals(grade) || "BA".equals(grade) || "3.5".equals(grade) || "3,5".equals(grade))
        return Grade.Ab;

    // ... more if statements here

    return null;
}

Seems like a great application, congrats!

\$\endgroup\$
1
\$\begingroup\$

Bug

When you are prompting for your 8th term/lessonTwo scores, you are reusing your creditsOne array when it should be creditsTwo. This is hard to pick up at first glance because they only differ in being one element short. If you need to change the array values between the two, you'll then realize the calculations will be incorrect.

Hardcoding

In addition to assigning total = 235, you should also assign 139 and 142 to a variable which communicates what they are.

try-with-resources

For safe and efficient handling of the Scanner resource, you can use it with try-with-resources:

public static void main(String[] args) {
    // variables declarations
    try (Scanner scanner = new Scanner(System.in)) {
        total += getScore("7th", scanner, lessonOne, creditOne);
        System.out.printf("Your cumulative GPA: %.3f", total / 139);
        total += getScore("8th", scanner, lessonTwo, creditTwo);
        System.out.printf("Your final GPA: %.3f", total / 142);
    }
}

private static double getScore(String prompt, Scanner scanner, 
                                String[] lessons, int[] credits) {
    System.out.printf("Enter your %s term notes:", prompt);
    double score = 0;
    for (int i = 0; i < credits.length; i++) {
        System.out.print(lessons[i] + ": ");
        // borrowing @stephenwebber's answer
        score += parseGrade(scanner.getLine()).toValue() * credits[i];
    }
    return score;
}

Here, we rely on method extraction to loop through your lessonOne, creditOne/lessonTwo, creditTwo arrays in a consistent manner, and some nice bits of @stephenwebber's answer to formulate the computation of a total score per term. A step further will be to consider using a Map to represent the relationship between a lesson and its credit, and this will be left as an exercise for the reader. :)

\$\endgroup\$
  • 1
    \$\begingroup\$ I shouldn't do copy paste that is why i made the mistake forgetting 'creditsTwo[]' array. I didn't understand when you say mapping. I think it is a feature of Java right? I will look for it and i appreciate your helps :) \$\endgroup\$ – Uygar Uçar Aug 28 '16 at 17:18
  • \$\begingroup\$ @ucha, yup I'm referring to this Map. :) \$\endgroup\$ – h.j.k. Aug 30 '16 at 15:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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