Cannot print out the right sum

Question

I have assigned a letter grade to each student based on his/her grade. And I want to count the total number of students for each letter grade. But for some reasons, the result turns total wrong. Can anyone tell me what parts I did wrong? Thank you!

String letter ="A";
int letterA=0;
int letterB=0;
int letterC=0;
int letterD=0;
int letterF=0;
int a=0;
int b=0;
int c=0;
int d=0;
int f=0;
for (int row=0;row<100;row++){ //the outer loop, have 100 rows to go through 
   for ( int column=0;column<words.length;column++) { 
       if(words[column].equals(table[row][column])){ // compare two arrays
            count++; }// add the score for each student if he/she is right
   }
     if (count >=18)
         letter=("A");
     if(count>=16 && count<18)
         letter=("B");
     if(count>=14 && count<16)
         letter=("C");
     if(count>=12 && count<14)
         letter=("D");      
     if(count<12)
         letter=("F");    
    System.out.println("Student Grade: "+letter+"\t");   
     count=0; // make sure the count will go back to 0, and run the loop again 

if (letter.equals("A"))
     letterA++;
     a+=letterA;}  
if (letter.equals("B"))
     letterB++;
     b+=letterB;
if (letter.equals("C"))
     letterC++;
     c+=letterC;
if (letter.equals("D"))
     letterD++;
     d+=letterD;
if (letter.equals("F"))
     letterF++;
     f+=letterF;
System.out.print("Question A "+a);
System.out.print("Question B "+b);
System.out.print("Question C "+c);
System.out.print("Question D "+d);
System.out.print("Question F "+f);
 }

Answer 1

Always use braces, even for single statements:

if (letter.equals("A"))
     letterA++;
     a+=letterA;}  
if (letter.equals("B"))
     letterB++;
     b+=letterB;
if (letter.equals("C"))
     letterC++;
     c+=letterC;
if (letter.equals("D"))
     letterD++;
     d+=letterD;
if (letter.equals("F"))
     letterF++;
     f+=letterF;

Should always be:

if ("A".equals(letter)) { letterA++; }
else if ("B".equals(letter)) { letterB++; }
else if ("C".equals(letter)) { letterC++; }
else if ("D".equals(letter)) { letterD++; }
else if ("F".equals(letter)) { letterF++; }
else { throw new RuntimeException("Invalid Letter " + letter); }

Use single line statements if you want them visually compact, but still use the braces as they guarantee the intent of what is to be done in a given block, they also act as documentation of that intent for people in the future ( people includes you ) to know what is going on.

I see no valid reason for the single letter variables they are never used and I do not understand why they are there?

General Critique:

Always use braces:

Leaving out braces means only the first statement after the if is executed when the if matches, the next line(s) are always executed.

if (letter.equals("A"))
     letterA++;
     a+=letterA;

is actually

if (letter.equals("A")) { letterA++; }
a+=letterA;

Which means that line outside the braces will always get executed no matter what the expression inside the if test evaluates to. The indention of the second line is conflating that statement as part of the if block and it is not.

There is absolutely nothing to gain by leaving out braces and everything to lose.

Neat code is easy to read and maintain and shows you care:

Always format your code consistently and not so densely so you can tell what is going on and what was wrong with the braces missing version.

Look at the best advertising it has plenty of white space, clean formatted code should have plenty of consistent white space as well so that our brains can quickly pattern match and scan for relevant things like matching pairs of braces.

Clean formatted code is just a keystroke away in all IDEs worth using.

Clean code shows you care about what you are doing and makes your question more appealing, which means others will care about their answer just as much.

Clean code earns you respect from your peers that know what they are looking at and sets you apart from those that do not care or know what they are looking at.

Most Importantly Clean Code is easier to reason about and has less bugs, no subtle bugs and is orders of magnitude easier to maintain.

Always cover all the conditions:

Always use if/else if/else with mutually exclusive tests.

If all the if clauses are mutually exclusive and you match the first one, all the rest are still evaluated for no reason with the if/if/if/if structure. if/else if/else if/else if/else only evaluates until something matches or nothing matches.

Without else you do not cover all possible cases that do not match, which is usually and error that will just occur silently without the else.

Do not just cover the happy path, cover the exceptional path, but do it in the least defensive manner possible.

Explicit is always better than Implicit!

Avoid == null checks; avoid null completely:

Always compare literals to variables with .equals() to avoid possible NullPointerExceptions.

Avoid using null references completely, it is possible in every case, even the cases that it seems like a legitimate reason.

If Tony Hoare, the inventor of them thinks it is a mistake who is to argue.

Always Name your variables descriptively!

I am not sure what letterA is supposed to represent anymore than what a is supposed to represent. So no one can tell you if these are correct because no one knows for sure what they represent semantically.

No Unnamed Numerical Constants ( Magic Numbers ):

final int A_GRADE = 18;
final int B_GRADE = 16;
final int C_GRADE = 14;
final int D_GRADE = 12;

Given the way they are used the names arguably could be even better like MINIMUM_A_GRADE, but that is opinion based, the lesson is avoid magic numbers.

Do range checks in the same direction:

Do range checks in the same direction so that the variable is visually in the middle of the comparison. This makes it harder to break the logic later on and is self documenting that it is a range check.

if (count >= A_GRADE) { /* omitted */ }
else if (B_GRADE <= count && count < A_GRADE) { /* omitted */ }
else if (C_GRADE <= count && count < B_GRADE) { /* omitted */ }
else if (D_GRADE <= count && count < C_GRADE) { /* omitted */ }
else /* isF */ { /* omitted */ }

Which one is easier to reason about and maintain?

Do not hesitate to have many small methods if they make the code more self documenting:

private static boolean isA(final int count) { return count >= A_GRADE; }
private static boolean isB(final int count) { return B_GRADE <= count && count < A_GRADE; }
private static boolean isC(final int count) { return C_GRADE <= count && count < B_GRADE; }
private static boolean isD(final int count) { return D_GRADE <= count && count < C_GRADE; }

then you will have the following:

if (isA(count)) { /* omitted */ }
else if (isB(count)) { /* omitted */ }
else if (isC(count)) { /* omitted */ }
else if (isD(count)) { /* omitted */ }
else /* isF */ { /* omitted */ }

Which one is more obvious and self documenting, thus more maintainable?

DRY - Don't Repeat Yourself:

Logically if (count >= A_GRADE) { letter = "A";} is exactly the same as if ("A".equals(letter)) { /* do stuff */ } so this is duplicated logic.

Instead of assigning a letter than checking that again, just put the logic in the original check.

if (count >= A_GRADE) { /* do stuff */ }
else if (B_GRADE <= count && count < A_GRADE) { /* do stuff */ }
else if (C_GRADE <= count && count < B_GRADE) { /* do stuff */ }
else if (D_GRADE <= count && count < C_GRADE) { /* do stuff */ }
else { /* omitted */ }

I see no valid reason for the single letter variables they are never used and I do not understand why they are there?

Duplicate logic means multiple places to have errors and multiple places to edit to fix bugs, save yourself time and effort and follow the DRY principle.

Move large logic blocks to method calls:

When do stuff is more than a few lines, refactor it to a method call.

if (count >= A_GRADE) { recordMarkA(); }
else if (B_GRADE <= count && count < A_GRADE) { recordMarkB(); }
else if (C_GRADE <= count && count < B_GRADE) { recordMarkC(); }
else if (D_GRADE <= count && count < C_GRADE) { recordMarkD(); }
else { recordMarkF(); }

More small methods with descriptive names is always better than large monolithic blocks of inline code.

This question shows substantial effort and a genuine desire to learn:

So I crafted up what I would want a complete solution ( provided the partial/incomplete code ) to look like.

Q34081279.java

public class Q34081279
{
    final static int A_GRADE = 18;
    final static int B_GRADE = 16;
    final static int C_GRADE = 14;
    final static int D_GRADE = 12;

    public static void main(final String[] args)
    {
        final String[] words = new String[]{}; /* this is just a placeholer, not provided in question */
        final String[][] table = new String[][]{}; /* this is just a placehoder, not provided in question */

        int markA = 0;
        int markB = 0;
        int markC = 0;
        int markD = 0;
        int markF = 0;

        for (int row = 0; row < 100; row++)
        {
            int count = 0;
            for (int column = 0; column < words.length; column++)
            {
                if (words[column].equals(table[row][column])) { count++; }
            }
            if (count >= A_GRADE) { System.out.format("%d = A", count); }
            else if (B_GRADE <= count && count < A_GRADE) { System.out.format("%d = B", count); }
            else if (C_GRADE <= count && count < B_GRADE) { System.out.format("%d = C", count); }
            else if (D_GRADE <= count && count < C_GRADE) { System.out.format("%d = D", count); }
            else { System.out.format("%d = F", count); }
            System.out.println();
        }

        System.out.println(String.format("Question A %d", markA));
        System.out.println(String.format("Question B %d", markB));
        System.out.println(String.format("Question C %d", markC));
        System.out.println(String.format("Question D %d", markD));
        System.out.println(String.format("Question F %d", markF));
    }
}

Answer 2

@Buddy is on the right rack but it's more than that. All of the if statements with more than one line need to have braces.

Otherwise only the first line is read by the compiler. According to Oracle:

Deciding when to omit the braces is a matter of personal taste. Omitting them can make the code more brittle. If a second statement is later added to the "then" clause, a common mistake would be forgetting to add the newly required braces. The compiler cannot catch this sort of error; you'll just get the wrong results.

eg:

if (letter.equals("A")) {
  letterA++;
  a+=letterA;
}  
if (letter.equals("B")) {
  letterB++;
  b+=letterB;
}