How's my Java code?

[michaelp]

I'm just beginning to learn Java, as a student who was going to participate in a regional programming competition has other commitments on the day of the competiton, so the teacher who is helping out the team is thinking about putting me in their place.
Most of the students, AFAIK, are at a higher level of programming skill than I am, and also use Java, which I have not looked at until yesterday. He suggested I look at Java, because if I got stuck and was not using Java, they would most likely not be able to help me with the problem.
So, this weekend, I am looking at Java. Because I have experience with C-style languages, it's been pretty easy so far.
If someone could critic the code for a solution I wrote for a problem, that would be great, as I'm not really sure if the things I am doing are the best way to do them.

The problem:

Quote:

Write a subprogram that tracks marks for a test. The subprogram should ask the user for the number of marks to be entered. This entry should be checked for a valid number, or else give the user an error message and ask them to reenter the mark. Next, the user should begin to enter the marks (0-100). Any invalid marks should be rejected and an error message should appear and the user should reenter the mark. As the marks are entered they should be classified according to letter grades( A 100-80, B 79-70, C 69-60, D 59-50, F 49-0). Lastly, the subprogram should display the number of marks in each letter grade and overall class average.

My solution:

Java:

import java.io.*; public class HelloWorldApp {         public static void main(String[] args) throws IOException         {                 BufferedReader cin = new BufferedReader( new InputStreamReader( System.in ) );                 int numMarks;                 System.out.print( "Enter the number of marks to enter: " );                 numMarks = Integer.parseInt( cin.readLine() );                 if( numMarks <= 0 )                 {                         do                         {                                 System.out.print( "Invalid number of marks. Reenter: " );                                 numMarks = Integer.parseInt( cin.readLine() );                         } while( numMarks <= 0 );                 }                                 int total = 0;                 double average = 0;                 int[] marks = new int[5]; //[0] = A, [1] = B, etc                 int mark;                                 int count = 0;                 while( count < numMarks )                 {                         System.out.print( "Enter mark #" + ( count + 1 ) + ": " );                         mark = Integer.parseInt( cin.readLine() );                         if( mark >= 0 && mark <= 100 )                         {                                 if( mark <= 100 && mark >= 80 )                                 {                                         marks[0]++;                                 }                                 else if( mark <= 79 && mark >= 70 )                                 {                                         marks[1]++;                                 }                                 else if( mark <= 69 && mark >= 60 )                                 {                                         marks[2]++;                                 }                                 else if( mark <= 59 && mark >= 50 )                                 {                                         marks[3]++;                                 }                                 else                                 {                                         marks[4]++;                                 }                                 count++;                                 total += mark;                         }                               }                 System.out.println( "A\tB\tC\tD\tF" );                 for( int i = 0; i < 5; i++ )                 {                         System.out.print( marks[i] + "\t" );                 }                 average = (float)total / (float)numMarks;                 System.out.println();                 System.out.println( "Class average: " + average );         } }

Some things I'm wondering:
-Is Integer.parseInt() the correct way to read integer input from the command line?
-In this line:

code:

average = (float)total / (float)numMarks;

Is it necessary to cast total and numMarks to floats? I do this because I read something that said it would perform integer division because both operands are integers.
-Is the array use okay? (I am aware of ArrayList. Would ArrayList be the best way to keep track of data, or are there are other containers?)

Links to tutorials on Java would be nice as well.

Comments much appreciated.

[unoho]

this is similar to array exercise we r doing in class
i wonder if we r frm the same school

[michaelp]

Possibly.
I go to Thornlea SS. How about you?

[rdrake]

michaelp @ Sat Apr 10, 2010 1:32 pm wrote:

-Is Integer.parseInt() the correct way to read integer input from the command line?

Well, the cin.readLine() is actually doing the reading in from the command line. Integer.parseInt() is indeed the correct way to grab an integer from a string.

michaelp @ Sat Apr 10, 2010 1:32 pm wrote:

-In this line:

code:

average = (float)total / (float)numMarks;

Is it necessary to cast total and numMarks to floats? I do this because I read something that said it would perform integer division because both operands are integers.

You only really need to cast one of them.

michaelp @ Sat Apr 10, 2010 1:32 pm wrote:

-Is the array use okay? (I am aware of ArrayList. Would ArrayList be the best way to keep track of data, or are there are other containers?)

Arrays are fine, magic numbers are not. Instead of using 0, 1, etc, I'd use a enumeration.

Java:

enum Grades { A, B, C, D, F }; ... if( mark <= 100 && mark >= 80 ) {     marks[Grades.A]++; }

It's been a while since I've been subjected to Java, but that or something similar should work.

[DemonWasp]

Critiques:

• It's usually poor form to import something like java.io.*, though it is more expedient. It's usually better to spell out each import (java.io.BufferedReader, java.io.InputStreamReader...) separately. Competent IDEs (Eclipse is an excellent one) will do this for you. This is a minor consideration.
  
• Integer.parseInt() will throw a NumberFormatException if the input isn't an integer (for example, "abcd" or "1.25"), which isn't caught by your code. You want to use a try { ... } catch ( NumberFormatException e ) { ... } block to deal with this.
  
• Look into using java.util.Scanner, which is newer and provides a lot of helpful facilities like nextInt() and hasNextInt().
  
• If you know the size of storage required, as you do in this problem, then arrays are great to use. If you don't necessarily know the size of the storage before you need the storage, then you want to use the List<type> interface. ArrayList<type> implements this interface, which means you can do either of the following:
  

  code:

  List<Integer> marks = new ArrayList<Integer>(); List<Integer> marks = new LinkedList<Integer>();

  
  Though in general, ArrayLists are preferred for their speed in iteration (going through the list), LinkedLists can be preferable if you're doing a lot of insert-before-the-end operations, which ArrayLists can be slow at.
  
• Typically, your while ( count < numMarks ) loop would be implemented as a for loop, though this is a style consideration.
  
• Your code can make use of the short-circuit properties of if statements - the first if/else if that evaluates to true is executed and no others. In the if ( mark >= 0 && mark <= 100 ) block, you know that mark is in the range [0,100], so you can skip testing against that again. In the inner if, you can re-write the code to only check one of the bounds, if you do it cleverly:
  

  code:

  if ( mark >= 80 ) {     // A } else if ( mark >= 70 ) {     // B } else if ( ... ) {     ... }

  
  
• You should only have to cast one of total and numMarks to float, because division between a float and an int will return a float, but there's really no harm in casting both.
  

Sun's Java Tutorials are very comprehensive, accurate, and easy to understand. You should probably also bookmark the Java API

Addendum: if you are currently using Ready to Program, stop that immediately and get a better solution. Notepad + cmd.exe is a superior solution, though I would recommend something like Dr. Java or Eclipse or Netbeans or Notepad++ and the command-line.

[wtd]

Java:

if( numMarks <= 0 )                 {                         do                         {                                 System.out.print( "Invalid number of marks. Reenter: " );                                 numMarks = Integer.parseInt( cin.readLine() );                         } while( numMarks <= 0 );                 }

Do you need both the if and the do...while?

[michaelp]

Thank you for all the comments.

rdrake: The enumeration does not work exactly like that, because when putting Marks.A in the brackets, there is an error about it not being an integer, which I don't know how to solve. Instead, I used 5 constants.

DemonWasp: -One reason why I used import java.io.* was because I wasn't really sure of the names of what I should import for using BufferedReader and InputStreamReader.
-Thanks for the lists syntax, I'm sure it will come in handy in the future.
-I used a counter and a while loop because I only wanted to increment count when a proper value was entered. I felt like using a for loop and decrementing the counter when a bad value was entered wasn't right/proper.
-Currently, I'm using Ubuntu and use gedit and the command line.

wtd: Not really. The reason why I did that was so that a new message pops up when invalid input is entered instead of the same prompt every time.
My new code( for the part wtd mentioned ) looks like this:

Java:

int numMarks = 0; String prompt = "Enter the number of marks to enter: "; do {         System.out.print( prompt );         try         {                 numMarks = Integer.parseInt( cin.readLine() );                 prompt = "Invalid number of marks. Reenter: ";         }         catch( NumberFormatException e )         {                 System.out.println( "Could not convert to an integer." );         }                         } while( numMarks <= 0 );

[DemonWasp]

You can, for reference, leave out any part of the for loop. For example, you can manually increment:

code:

for ( int i = 0; i < max; ) {     // Some logic here...     ++i; }