views:

64

answers:

2

I am almost done with this but there are 2 things that are stumping me in my code. When I query a user for the test score, if the score is not within the 0-100 range, I want to not accept it and then tell them why and ask for another input. I also want to print the letter grade for their average next to their average score. For some reason my If logic statement isn't working when I try to check to make sure the score entered is within 0-100. Also I can't figure out how to get the letter grade to print, but I am not getting any error output so I think that I am on the right track. I think that I could mainly use pointers on my while looping to check for the number being in the range of 0-100. I would greatly appreciate it. Here is my code:

    import java.text.DecimalFormat;
import java.util.Scanner;
public class GradeReport 
{
 String name;
 int score1, score2, score3;
 double average;
 String grade;
 public GradeReport()  //creates the first constructor
 {
  Scanner sc = new Scanner (System.in);

  System.out.println ("Enter student's name: ");
  name = sc.nextLine();

  System.out.println ("Enter first grade: "); //try while loops to get grade in between 0-100
    score1 = sc.nextInt();
    while
        (score1 <0 || score1 > 100);
    System.out.println("please enter a grade 0-100"); //checks that score is inclusive 1-100

    System.out.println ("Enter second grade: ");
    score2 = sc.nextInt();
    while
        score2 <0 || score2 > 100;
    System.out.println("please enter a grade 0-100");//checks that score is inclusive 1-100

    System.out.println ("Enter third grade: ");
    score3 = sc.nextInt();
    while
        score3 <0 || score3 >100;
    System.out.println("please enter a grade 0-100");//checks that score is inclusive 1-100
 }
 public GradeReport (String v1, int v2, int v3, int v4)
 {
  name = v1;  //these are to initialize the variables so that I don't get null for the second set of results.
  score1 = v2;
  score2 = v3;
  score3 = v4;
 }
 public void calculateAvg()
 {
  average = (double)((score1 + score2 + score3) / 3.0);


 }
 public String calculateGrade()
 {
  if (average >= 90)
   grade = "A";
  else if (average >= 80)
   grade = "B";
  else if (average >= 70)
   grade = "C";
  else if (average >= 60)
   grade = "D";
  else
   grade = "F";
  return grade;
 }

 public String toString()
 {
  DecimalFormat fmt = new DecimalFormat ("0.00"); //to format average to 2 decimal places
  String gradeReport = name + "\n " + Double.toString(score1) + "\t" + Double.toString(score2)+ "\t" + Double.toString(score3) + "\n" + fmt.format(average) + grade;
  return gradeReport;
 }

 public static void main (String[] args)
 {
  GradeReport gr1 = new GradeReport();
  GradeReport gr2 = new GradeReport("Col Een", 76, 76, 75);
  gr1.calculateAvg();
  gr1.calculateGrade();
  gr2.calculateAvg();
  gr2.calculateGrade();
  System.out.println(gr1);
  System.out.println(gr2);
 }

}
A: 

Some of your code looks uncompilable to me, but that aside, consider the difference between the following three lines of code:

while (score1 <0 || score1 > 100)

compared to

if (score2 <=0)

and

if (score3 <=0)

I think you'll find part of your problem there. For validation, consider the following pattern:

do {
  //collect input
} while (something that's false if input is invalid);
jball
Ah. So you suggest using a do while loop instead of a while loop. I am having a hard time with learning looping so thanks for the help. I changed my code above but I think I changed it to something that might be more wrong than it was before.
Josh
The loop shown above is the syntax that you should have in your code, including the `{ }` around the code soliciting user input, and the `( )` around the expression evaluating the input after the `while` keyword at the end of the loop.
jball
+1  A: 

Some comments...

indentation

Please try to be consistent with your indentation style. It makes your code much easier to read.

whitespace

Be careful how you manage your whitespace. When you read a book or a magazine or a web page, blank spaces are used to separate ideas - paragraphs, sections, etc. Similarly, empty space should be used to separate your functions, or ideas within functions. For example:

void f1()
{
  do();
  domore();
}


void f2()
{
  doAnotherThing();
  andYetAnother();
}


void f3()
{
  do1();
  do2();

  do3();
  do4();
}

Note that it's easy to see that there are 3 separate functions, and the third function has two separate groups of things it's doing - do1() and do2() are separated from do3() and do4() with empty space, visually indicating that there's something similar about do1() and do2(), something else similar about do3() and do4(), and that 1&2 are somehow different from 3&4.

If that doesn't make sense, feel free to ignore :) (but I would suggest you read a basic visual design book)

braces

It's a good idea to get into the habit of using braces around all conditional blocks - it makes it very clear where the block begins and ends. For example,

if(condition)
   line1;
   line2;

is different from

if(condition)
{
   line1;
   line2;
}

In the first case, line1 will execute if and only if the condition is true, but line2 will evaluate no matter what - the indentation is deceiving. In the second case, both line1 and line2 will execute if and only if the condition is true.

In the first case, the intention is unclear - did the original developer screw up indentation (for the pedantic, ignore languages that use indentation to manage loops)? Or did s/he forget the braces? If the first case were written as follows, we'd know the answer:

if(condition) {
  line1
}
  line2

Of course, if indentation were consistent through the file, the intention of the following code would be clear, as well:

if(condition)
  line1
line2

infinite loop

Note the trailing semicolon that you have on

 while
        (score1 <0 || score1 > 100);

The trailing semicolon ends the block. If score1 is invalid, the loop will never exit.

compilation

I'm not sure that

while
        score2 <0 || score2 > 100;

is valid code. You should place parens around the condition. You've also got the infinite loop problem with the trailing semicolon, again.

getting grades

When you ask for grades, your code currently looks like

score = readLine()
while(...)
  System.out.println(...)

This means that you read user input, then enter the loop, in which you print a message. Remember: the loop starts where the while starts, so reading input never happens after the first iteration. You need to read the value the user writes every iteration of the loop.

score = readline()
while(score is invalid)
{
   print error
   score = readline()
}

variable naming

Ignore this section if it doesn't make sense right now - consider it a bad introduction to stuff you'll learn later.

If you have the option, you should always name your variables meaningfully. The GradeReport constructor has 4 variables, whose purpose is totally unclear if you don't have access to the source code:

public GradeReport (String v1, int v2, int v3, int v4)

You could use the same name for the variable as the class slot, and you can differentiate with the this keyword. If we first insert this for all class variables in the constructor, and leave the rest unchanged, it becomes:

 public GradeReport (String v1, int v2, int v3, int v4)
 {
  this.name = v1;  //these are to initialize the variables so that I don't get null for the second set of results.
  this.score1 = v2;
  this.score2 = v3;
  this.score3 = v4;
 }

and if we replace v1 with "name"...

 public GradeReport (String name, int v2, int v3, int v4)
 {
  this.name = name;  //these are to initialize the variables so that I don't get null for the second set of results.
  this.score1 = v2;
  this.score2 = v3;
  this.score3 = v4;
 }

and then replace v2 with score1...

 public GradeReport (String name, int score1, int v3, int v4)
 {
  this.name = name;  //these are to initialize the variables so that I don't get null for the second set of results.
  this.score1 = score1;
  this.score2 = v3;
  this.score3 = v4;
 }
atk
+1 for style tips and variable naming
jball
Thank you atk. I agree with jball that you did a really good and thorough job explaining all of this.
Josh