2
\$\begingroup\$

This program takes a file from a user, "example.txt". The file contains only positive integers, and the first interger in the file is stating how many intergers come after that integer (Ex. 3, 45, 98, 102) It is safe to assume everything in the users file is properly formatted and they put .txt at the end. The program is pretty simple for the most part, it was mainly to practice Array's, but I'm curious if I can cut this down a bit, or if I over-complicated anything. This code works by the way.

import java.util.*;
import java.io.*;
import java.lang.*;

public class TestScoresAndSummaryStatistics
{
public static void main(String[] args) throws IOException 
{

int i = 0;
Scanner keyboard = new Scanner(System.in);

System.out.println("Enter the name of the file");
String fileName = keyboard.nextLine();
File inputFile = new File(fileName);
Scanner data = new Scanner(inputFile);


//declaring that whatever the first number in the document is is how many indices the document has
int scores[] = new int[data.nextInt()];

while (data.hasNext())
{
  scores[i] = data.nextInt();
  i++;
}

Arrays.sort(scores);


//Highest and Lowest score
System.out.println("Lowest score: " + scores[0]);
System.out.println("Highest score: " + scores[(scores.length - 1)]);


//Calculating Mean
double gradesTotal = 0;
for (i=0; i<scores.length; i++)
{
  gradesTotal = gradesTotal + scores[i];
}
double mean = gradesTotal/scores.length;
System.out.println("Mean score: " + mean);


//Calculating Standard Deviation
double variance = 0;
for (i=0; i<scores.length; i++)
{
  variance = variance + (Math.pow((scores[i] - mean), 2));  
}
variance = variance / scores.length;
double standardDeviation = Math.sqrt(variance);
System.out.println("Standard Deviation: " + standardDeviation);


//Finding Median score
double median;
if (scores.length % 2 == 0)
  median = ((scores[(scores.length/2) - 1]) + scores[(scores.length/2)]) / 2;
else
  median = scores[(scores.length/2)];
System.out.println("Median score: " + median);


//Finding how many number of A's, B's, etc
int gradeA = 0;
int gradeB = 0;
int gradeC = 0;
int gradeD = 0;
int gradeF = 0;


for (i=0; i<scores.length; i++)
{
  if (scores[i] >= 90)
  {
    gradeA++; 
  }
  else if (scores[i] <= 89 && scores[i] >=80)
  {
    gradeB++; 
  }
  else if (scores[i] <= 79 && scores[i] >=70)
  {
    gradeC++; 
  }
  else if (scores[i] <= 69 && scores[i] >=60)
  {
    gradeD++; 
  }
  else
  {
    gradeF++; 
  } 
}

System.out.println("You have recieved " + gradeA + " A's, " + gradeB + " B's, " + gradeC + " C's, " + gradeD +
                   " D's, and " + gradeF + " F's");


}
}
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Some general issues first:

  • Indent methods and method blocks. Makes your code more readable
  • Avoid resource leaks. Close the Scanners in a finally block.

This program has only one method. Split the functionality into many small methods, one for each purpose. For example: reading the scores from the file. This is easy to extract as a method:

    int[] scores = readScoresFromFile(fileName);

...

private static int[] readScoresFromFile(String fileName) throws FileNotFoundException {
    File inputFile = new File(fileName);
    Scanner data = new Scanner(inputFile);
    try {
        //declaring that whatever the first number in the document is is how many indices the document has
        int scores[] = new int[data.nextInt()];

        int i = 0;
        while (data.hasNext())
        {
            scores[i] = data.nextInt();
            i++;
        }

        Arrays.sort(scores);
        return scores;
    }
    finally {
        data.close();
    }
}

I also added the finally block, so the Scanner gets closed. data is a bad name (it could by anything), so let's call it just scanner. Simple and understandable. The method is still too long, so I'd extract one more method, so that the try{} block contains just readScoresFromScanner(scanner);.

As you are using arrays, you need to know the total number of scores in advance. That's not necessary. Collections make your life easier. Assuming you remove the number-of-scores from your input file, you can write:

private static List<Integer> readScoresFromScanner(Scanner scanner) {
    List<Integer> scores = new ArrayList<>();
    while (scanner.hasNext())
    {
        scores.add(scanner.nextInt());
    }
    Collections.sort(scores);
    return scores;
}

Of course, now you need to make some changes to the rest of the code. It will make the code more readable:

    List<Integer> scores = readScoresFromFile(fileName);
    //Highest and Lowest score
    System.out.println("Lowest score: " + Collections.min(scores));
    System.out.println("Highest score: " + Collections.max(scores));

With good refactoring, the comments become unnecessary:

    System.out.println("Mean score: " + calculateMean(scores));

The method becomes very understandable; consider using enhanced loops:

private static double calculateMean(Collection<Integer> scores) {
    double gradesTotal = 0;
    for (int score : scores) {
        gradesTotal = gradesTotal + score;
    }
    return gradesTotal / scores.size();
}

With Java 8, you can also write:

private static double calculateMean(Collection<Integer> scores) {
    return scores
        .stream()
        .mapToInt(Integer::intValue)
        .average()
        .getAsDouble();
}

The code that sums up the grades is very verbose and redundant; try to avoid if-else chains and replace them with, for instance, loops. Use an array or list instead of gradeA, gradeB, etc. A hint:

private static int gradeByScore(int score) {
    int[] scoreForGrade = { 90, 80, 70, 60 };
    for (int grade = 0; grade < scoreForGrade.length; grade++) {
        int requiredScore = scoreForGrade[grade];
        if (score >= requiredScore) {
            return grade;
        }
    }
    return 4; // grade F
}
\$\endgroup\$
1
\$\begingroup\$

I am also a beginner just like you and since Christoph has already covered most of the functional part i will just comment about the style.

Java 'style guides' all agree that in Java, the { opening brace should be at the end of the line declaring a code block. So, for example, this code

while (data.hasNext())
{
    scores[i] = data.nextInt();
    i++;
} 

should be :

while (data.hasNext()){
    scores[i] = data.nextInt();
    i++;
}

and likewise everywhere in your code

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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