4
\$\begingroup\$

I wanted to get some feedback here on my solution, because the course I am using is not allowing me to submit any answers.

Objectives:

Write a Java program that:

  1. Create a grades.txt file with some grades inputted in
  2. Reads these grades from the file and stores it into an ArrayList
  3. After storing all elements into the ArrayList return the max, min, and average grade of the list
  4. Return an Arraylist without any duplicate grades. All duplicated grades must be removed

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


public class Main {

    public static void main(String[] args) throws IOException {
       File file = new File("grades.txt");

      PrintWriter output = new PrintWriter(file);
      output.println("12.5");
      output.println("19.75");
      output.println("11.25");
      output.println("10");
      output.println("15");
      output.println("13.25");
      output.println("14");
      output.println("9");
      output.println("10");
      output.println("19.75");
      output.close();

      // Reading from the file //

        ArrayList<Double> gradesString = new ArrayList<Double>();

        Scanner input = new Scanner(file);

        while(input.hasNext())
        {
            String line = input.nextLine();
            gradesString.add(Double.parseDouble(line));
        }

        double result = 0;

        for(Double i : gradesString)
        {
            result += i;
        }

        double mean = result / gradesString.size();

        LinkedHashSet<Double> hashSet = new LinkedHashSet<>(gradesString);

        ArrayList<Double> gradesWithoutDuplicates = new ArrayList<Double>(hashSet);


        System.out.println("The grades are: " + gradesString);
        System.out.println("The highest grade is: " + Collections.max(gradesString));
        System.out.println("The lowest grade is: " + Collections.min(gradesString));
        System.out.println("The average is: " + mean);
        System.out.println("The grades list without duplicates is: " + gradesWithoutDuplicates);


    }

}

Output:

The grades are: [12.5, 19.75, 11.25, 10.0, 15.0, 13.25, 14.0, 9.0, 10.0, 19.75]
The highest grade is: 19.75
The lowest grade is: 9.0
The average is: 13.45
The grades list without duplicates is: [12.5, 19.75, 11.25, 10.0, 15.0, 13.25, 14.0, 9.0]
\$\endgroup\$
2
  • \$\begingroup\$ Please don't edit the question once answers are available. This would invalidate them. After you feel sufficient reviews are available, you could always ask a follow-up question with the improved code. \$\endgroup\$
    – dfhwze
    Commented Aug 4, 2019 at 8:48
  • \$\begingroup\$ You should have at least one method for each of your four bullet points. Reading a file, parsing a file, analyzing a data set, and deduplicating a dataset are very different responsibilities. They shouldn't all just be under one method (especially not main) \$\endgroup\$
    – Alexander
    Commented Aug 4, 2019 at 16:34

3 Answers 3

3
\$\begingroup\$

Exception handling

It is not customary to let the main method throw an exception. At the very least you can catch the base Exception class and use printStackTrace() to print the error message and related debug info.

IO handling

  1. This program only deals with one file. However, it is good practice to remember to close every external resource. That includes files, database connections (and other related resources, like prepared statements and result sets), network sockets, etc. not closing resources may result in resource leaks that can escalate to JVM crash. Java 7 introduced the try-with-resources construct that allows the compiler to automatically handle resource closure as well as error handling.

  2. Java 7 also introduced the "new io" java.nio package that improves IO handling in Java. the Files.readAllLines() is a convenient method that reads whole file into List of Strings (and also internally handles the external resource). (alas, no equivalent writeAllLines())

Naming conventions

gradesString is a List of Double items. the name is very confusing.

\$\endgroup\$
2
\$\begingroup\$

Review

Java is an object-oriented language, so I suggest you take advantage of that. I understand your main concern is to provide a solution to the problem, but a next step is to make it adhere to some quality attributes such as testibility, usability and readability. This way, you'll be able to write code that others can reuse and maintain.


Remarks

  • Algorithm logic is mixed with input parsing and output to end-user
  • Input file is fixed
  • No error handler provided (what if can't open file or bad input detected?)
  • Main method abused for creating the algorithm
  • No edge case handler provided (what if file is empty, what would min, max, mean be?)
  • No unit tests provided
  • Make variables that you don't change final. For instance, final double mean = result / gradesString.size();
  • Fix intendations

Proposed Changes

  • Split the current implementation into 3 methods: one for reading and parsing an input file, one for performing the algorithm and one for outputting results to the end-user.
  • Let main method call these 3 methods with a predefined input file and provide a simple error handler.
  • Create a custom class holding all the statistical results and return an instance of this class in the algorithm.
  • Provide unit tests for the algorithm for at least the happy path, but preferrably also for empty input.
\$\endgroup\$
2
\$\begingroup\$

As the previous answers https://codereview.stackexchange.com/a/225509/203649 and https://codereview.stackexchange.com/a/225510/203649 have already explained, the main problem of your code is about reading and writing to the file and the exception handling. Above my initialization of the filename and the creation of a double array in the main method:

String filename = "grades.txt";
File file = new File(filename);
double doubles[] = {12.5, 19.75, 11.25, 10, 15, 
                     13.25, 14, 9, 10, 19.75};

Now you can use the try-with-resources construct with PrintWriter:

 try(PrintWriter pw = new PrintWriter(file)) {
        for (Double d: doubles) {
            pw.println(d);
        }
  } catch (FileNotFoundException ex) {
        ex.printStackTrace();
  }

In this case you will decide what to do when you handle the FileNotFoundException exception, just for simplicity in this case I will print the error message.

After this you can start to read double values from the file;

ArrayList<Double> grades = new ArrayList<Double>();
  try {
      List<String> lines = Files.readAllLines(file.toPath());
      for (String line : lines) {
          grades.add(Double.parseDouble(line));
      }
  } catch (IOException ex) {
      ex.printStackTrace();
  }

Again, you have to decide what to do when you encounters a reading file error instead of just printing the error.

\$\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.