HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

GPA Calculator that takes letters or numbers as input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gpanumbersinputthattakescalculatorletters

Problem

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

Solution

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:

-
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.

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

Code Snippets

// 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(); 
}
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.
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
}

Context

StackExchange Code Review Q#139723, answer score: 4

Revisions (0)

No revisions yet.