5
\$\begingroup\$

This program takes scores for each student from a .txt file and calculates their averages, as well as the average for each assignment and the class overall. This program works properly, just curious if I can make it look more professional.

Scores.txt

5
6
95.2 89.1 98.0 78.9 100 67
100 99.6 100 100 90.1 82.2
100 85.5 85.1 74 81 79.4
98.6 71.5 68.9 62.4 56.9 0
100 100 100 88.3 91.6 81.3

StudentGradeBookScores.java

import java.util.*;
import java.io.*;
import java.text.*;
public class StudentGradebookScores {
  public static void main(String[]args) throws IOException {


    File inputFile = new File("scores.txt");
    Scanner data = new Scanner(inputFile);
    DecimalFormat averages = new DecimalFormat("#.#");
    int students = data.nextInt();
    int assignments = data.nextInt();
    double gradebook[][] = new double [students + 1][assignments + 1];

    //putting the scores into the array
    for (int i=0; i<students; i++) { //rows
      for (int j=0; j<assignments; j++) { //coloums
        gradebook[i][j] = data.nextDouble();
      }
    }

    //calculating the average assignment score for each student (coloums)
    for (int i=0; i<students; i++) {
      double studentTotal = 0;
      for (int j=0; j<assignments; j++) {
        studentTotal = studentTotal + gradebook[i][j];
      }
      gradebook[i][assignments] = studentTotal/assignments;
    }

    //calculating the average score of each assigment for all students (rows)
    for (int j=0; j<assignments; j++) {
      double assignmentTotal = 0;
      for (int i=0; i<students; i++) {
        assignmentTotal = assignmentTotal + gradebook[i][j];
      }
      gradebook[students][j] = assignmentTotal/students;
    }


    //printing the gradebook
    System.out.print("\t\t\t\tAssignment #:\n\t\t");
    for (int j=0; j<assignments; j++) {
      System.out.print((j+1) + "\t"); 
    }

    System.out.println("Avg");
    for (int i=0; i<students; i++) {
      System.out.print("Student #" + (i+1) + ":\t");
      for (int j=0; j<assignments; j++) {
        System.out.print(gradebook[i][j] + "\t");
      }
      System.out.println(averages.format(gradebook[i][assignments]));
    }
    System.out.print("Average\t\t");
    for (int j=0; j<assignments; j++) {
      System.out.print(averages.format(gradebook[students][j]) + "\t");
    }
    System.out.println();

    double overallAvg = 0;
    for (int i=0; i<students; i++) {
      overallAvg = overallAvg + gradebook[i][assignments];
    }

    System.out.println("\nOverall Average: " + averages.format(overallAvg/students));
  }
}
\$\endgroup\$
  • \$\begingroup\$ Can you use Java 8, or is this strictly < Java 8? \$\endgroup\$ – h.j.k. Mar 3 '15 at 5:36
  • 3
    \$\begingroup\$ Yes I can use Java 8! I would prefer to use the modern methods \$\endgroup\$ – Overclock Mar 3 '15 at 5:56
8
\$\begingroup\$

Declaring that main() throws IOException is probably the right thing to do for a simple program like this. A lot of beginners try to catch an exception that they don't have any idea how to handle properly.

That's a lot of code to be stuffed into a single main() function. Ideally, you should develop classes such that main() contains just a minimal amount of code, like this:

public static void main(String[] args) throws IOException {
    try (Scanner input = args.length > 0 ? new Scanner(new File(args[0])) :
                                           new Scanner(System.in)) {
        GradeBook book = new GradeBook(input);
        System.out.println(book);
        System.out.printf("Overall Average: %s\n", 
                          FMT.format(book.average()));
    }
}

Hard-coding an input filename makes it hard to reuse your code. I've modified it to read from either a filename specified on the command line or from System.in.

You should be able to avoid having to explicitly specify the dimensions of the data in the file.

Using Java 8 streams removes a lot of tedious looping from the implementation. Here is the rest of my suggested implementation:

import java.io.File;
import java.io.IOException;
import java.text.DecimalFormat;
import java.util.*;
import java.util.stream.IntStream;

public class GradeBook {
    private List<double[]> students;
    private static final DecimalFormat FMT = new DecimalFormat("#.#");

    public GradeBook(Scanner input) {
        this.students = new ArrayList<double[]>();
        while (input.hasNextLine()) {
            double[] student = Arrays.stream(input.nextLine().trim().split("\\s+"))
                  .mapToDouble(Double::parseDouble)
                  .toArray();
            students.add(student);
        }
    }

    public double getScore(int student, int assignment) {
        return this.students.get(student)[assignment];
    }

    public double averageForStudent(int student) {
        return Arrays.stream(this.students.get(student))
                     .average()
                     .getAsDouble();
    }

    public double averageForAssignment(int assignment) {
        return this.students.stream()
                   .mapToDouble((assignments) -> assignments[assignment])
                   .average()
                   .getAsDouble();
    }

    public double average() {
        return IntStream.range(0, this.students.size())
                        .mapToDouble((s) -> this.averageForStudent(s))
                        .average()
                        .getAsDouble();
    }

    public String toString() {
        StringBuilder out = new StringBuilder();
        int numAssignments = this.students.stream()
                                 .mapToInt((assignments) -> assignments.length)
                                 .max()
                                 .getAsInt();

        // Header
        out.append("\t\t\t\tAssignment #:\n\t\t");
        for (int a = 0; a < numAssignments; a++) {
           out.append(a + 1).append('\t'); 
        }
        out.append("Avg\n");

        // Body
        for (int s = 0; s < this.students.size(); s++) {
            out.append("Student #").append(s + 1).append(":\t");
            for (int a = 0; a < numAssignments; a++) {
                out.append(FMT.format(this.getScore(s, a))).append('\t');
            }
            out.append(FMT.format(this.averageForStudent(s))).append('\n');
        }

        // Footer
        out.append("Average\t\t");
        for (int a = 0; a < numAssignments; a++) {
            out.append(FMT.format(this.averageForAssignment(a))).append('\t');
        }
        out.append('\n');

        return out.toString();
    }

    public static void main(String[] args) throws IOException {
        try (Scanner input = args.length > 0 ? new Scanner(new File(args[0])) :
                                               new Scanner(System.in)) {
            GradeBook book = new GradeBook(input);
            System.out.println(book);
            System.out.printf("Overall Average: %s\n",
                              FMT.format(book.average()));
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$
//putting the scores into the array
for (int i=0; i<students; i++) { //rows
  for (int j=0; j<assignments; j++) { //coloums
    gradebook[i][j] = data.nextDouble();
  }
}  

Comments should describe why something is done. The code itself should describe what is done by using descriptive names vor variables etc.

Usually one would use i and j as looping variables, but if you feel that you need to add the comments //rows and //coloums where the later is spelled wrong too, you should consider to just name the looping variables right.

Giving the variables some space to breathe will also add readability to your code.

The default indentation level for java code is 4 spaces.

for (int row = 0; row < students; row++) {
    for (int column = 0; column < assignments; column++) {
        gradebook[row][column] = data.nextDouble();
    }
} 
\$\endgroup\$
3
\$\begingroup\$

Yet another take on (ab?)using the power of Java 8 streams...

@200_success's solution treats each line as a double[] array of scores, which works well enough. Taking the idea one step further, we can write our own Collector to directly map each score into a Result object, with references to a student and assignment.

public final class Result {
    private final int student;
    private final int assignment;
    private final double score;

    public Result(int student, int assignment, double score) {
        this.student = student;
        this.assignment = assignment;
        this.score = score;
    }

    public int getStudent() {
        return student;
    }

    public int getAssignment() {
        return assignment;
    }

    public double getScore() {
        return score;
    }
}

I suppose the Result class is straight-forward...

public final class ScoreCollector implements
        Collector<String, Collection<Result>, Collection<Result>> {

    private int numStudents = -1;
    private int numAssignments = -1;
    private int currentStudent = 0;

    @Override
    public Supplier<Collection<Result>> supplier() {
        return ArrayList::new;
    }

    @Override
    public BiConsumer<Collection<Result>, String> accumulator() {
        return (collection, line) -> {
            if (isRecord(line)) {
                collection.addAll(toResults(line));
            }
        };
    }

    @Override
    public BinaryOperator<Collection<Result>> combiner() {
        return null;
    }

    @Override
    public Function<Collection<Result>, Collection<Result>> finisher() {
        return accumulator -> accumulator;
    }

    @Override
    public Set<Characteristics> characteristics() {
        return EnumSet.noneOf(Characteristics.class);
    }

    private boolean isRecord(final String line) {
        if (numStudents == -1) {
            numStudents = Integer.parseInt(line);
            return false;
        } else if (numAssignments == -1) {
            numAssignments = Integer.parseInt(line);
            return false;
        }
        return true;
    }

    private Collection<Result> toResults(final String line) {
        final int student = ++currentStudent;
        final int[] assignment = new int[1];
        return Arrays.stream(line.split("\\s", numAssignments))
                .map(v -> new Result(student, ++assignment[0], Double.parseDouble(v)))
                .collect(Collectors.toList());
    }
}

Our custom ScoreCollector has to perform a few things:

  1. Parse and effectively ignore the first (number of students) and second (number of assignment) lines, optionally validating the later steps against these two values.
  2. Convert a line into a Collection<Result> while being aware of the current student (i.e. line) and assignment.
  3. Accumulate the resulting Collection<Result> into a final Collection<Result>.
  4. Returning the final Collection<Result>.

The accumulator() method performs the first two steps, through isRecord() validation and then converting with toResults(). Setting instance variables in isRecord() does seem a bit odd, I'm actually open to comments on this...

Referencing of the current student and assignment is squarely done in toResults(), by incrementing on a currentStudent variable and a temporary one-element assignment array. In the code above, line.split() is called with the second argument numAssignments, which is the total number of expected scores per student. This will expectedly throw an Exception if we have mismatched number of scores. You can omit this argument for flexibility.

Things get easier after coding ScoreCollector. The benefits of dealing with just a single Collection<Result> is that you can apply more streaming functions on it based on the values encapsulated in each Result object. For illustration, I went with an alternative output of showing the maximum and minimum scores per student and per assignment. The only calculation I did not derive is the "overall average", as I'm not too sure how that will be interpreted. Anyways, code please:

private static final String FORMAT = "%s #%d\tAverage: %.2f\tMax: %.2f\tMin: %.2f%n";

public static void main(String[] args) {
    try (final Stream<String> lines = Files.lines(Paths.get(ClassLoader.getSystemResource(
            "scores.txt").toURI()))) {
        final Collection<Result> result = lines.collect(new ScoreCollector());
        groupAndPrint(result, Result::getStudent, "Student");
        groupAndPrint(result, Result::getAssignment, "Assignment");
    } catch (IOException | URISyntaxException e) {
        throw new RuntimeException(e);
    }
}

@SuppressWarnings("boxing")
private static void groupAndPrint(final Collection<Result> collection,
        final Function<Result, ?> classifier, final String label) {
    collection
            .stream()
            .collect(
                    Collectors.groupingBy(classifier,
                            Collectors.summarizingDouble(Result::getScore)))
            .forEach(
                    (key, summary) -> {
                        System.out.printf(FORMAT, label, key, summary.getAverage(),
                                summary.getMax(), summary.getMin());
                    });
}

I am using try-catch on opening the file using Java 8's new Files.lines() method, and from that Stream of lines, we collect using our ScoreCollector. We then use a helper method groupAndPrint to group by our Collection<Result> with the desired classifer, which will be either student (Result::getStudent) or assignment (Result::getAssignment) in our case. Afterwards, it's just a quick call to Collectors.summarizingDouble() on our scores, and we can easily print out the average, maximum and minimum scores per student/assignment.

Student #1      Average: 88.03      Max: 100.00     Min: 67.00
Student #2      Average: 95.32      Max: 100.00     Min: 82.20
Student #3      Average: 84.17      Max: 100.00     Min: 74.00
Student #4      Average: 59.72      Max: 98.60      Min: 0.00
Student #5      Average: 93.53      Max: 100.00     Min: 81.30
Assignment #1   Average: 98.76      Max: 100.00     Min: 95.20
Assignment #2   Average: 89.14      Max: 100.00     Min: 71.50
Assignment #3   Average: 90.40      Max: 100.00     Min: 68.90
Assignment #4   Average: 80.72      Max: 100.00     Min: 62.40
Assignment #5   Average: 83.92      Max: 100.00     Min: 56.90
Assignment #6   Average: 61.98      Max: 82.20      Min: 0.00

I hope my usage of Java 8 streams is still a somewhat sane solution to your question, with more flexibility for alternative outputs...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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