HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Multiple file creation with sorted input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filewithinputmultiplesortedcreation

Problem

Is there a more efficient way to create the three files and write to those files using BufferedOutput and BufferedWiter I/O? The code works, but I thought there has to be a more compact way to accomplish this.

```
import java.nio.file.*;
import java.io.*;
import static java.nio.file.StandardOpenOption.*;
import java.util.Scanner;

public class SortStudent
{

public static void main(String[] args)
{
//variables for processing
Path fileOut1 = Paths.get("Honors_Students.txt");
Path fileOut2 = Paths.get("Good_Standing.txt");
Path fileOut3 = Paths.get("Probation_Students.txt");
String recOut;
String delimiter = ",";
Scanner input = new Scanner(System.in);
final int QUIT = 999;

//data class record
Student student = new Student();

//input.output may generate exceptions
try
{
//Setup the 3 files to be used for output
OutputStream output1 =
new BufferedOutputStream(Files.newOutputStream(fileOut1, CREATE));
OutputStream output2 =
new BufferedOutputStream(Files.newOutputStream(fileOut2, CREATE));
OutputStream output3 =
new BufferedOutputStream(Files.newOutputStream(fileOut3, CREATE));
BufferedWriter writer1 =
new BufferedWriter(new OutputStreamWriter(output1));
BufferedWriter writer2 =
new BufferedWriter(new OutputStreamWriter(output2));
BufferedWriter writer3 =
new BufferedWriter(new OutputStreamWriter(output3));

BufferedWriter writer = null;

//initial read for indefinite repetition
System.out.print("Enter Student's ID number or 999 to quit: ");
student.setID(input.nextInt());

//sentinel controlled loop
while (student.getID() != QUIT)
{

Solution

Formatting

Your if statement block should use braces { ... } to better identify the scope of each clause as a good practice. Also, you can consider renaming setgpa(double)/getgpa() to setGpa(double)/getGpa() respectively.

try-with-resources

Since Java 7, you should be using try-with-resouces for safe and efficient handling of your I/O resources:

private static final Path HONORS = Paths.get("Honors.txt");
private static final Path GOOD_STANDING = Paths.get("GoodStanding.txt");
private static final Path PROBATION = Paths.get("Probation.txt");
private static final String DELIMITER = ",";
private static final int QUIT = 999;

public static void main(String[] args) {
    try (PrintWriter honorsWriter = new PrintWriter(HONORS.toFile());
         PrintWriter goodStandingWriter = new PrintWriter(GOOD_STANDING.toFile());
         PrintWriter probationWriter = new PrintWriter(PROBATION.toFile());
         Scanner scanner = new Scanner(System.in)
    ) {
        // ...
    } catch (IOException e) {
        throw new RuntimeException(e);
    }
}


I have taken the liberty to declare your file paths as private static final constants outside of the main() method, again as a good practice. Also, I have opted to use a PrintWriter, which is buffered, so that I can make use of its println(String) method.

Determining the student's details and ranking

You are creating only one Student instance, and then repopulating its details for each input. Ideally, you should be creating a new one each time, so that they can all go into a Collection in the future for further processing. Right now, you probably can settle for temporary variables in order to print them afterwards.

Assuming we are sticking with your approach, you can hide the Student creation within its own method, e.g. getStudent(Scanner). The idea is that given a Scanner instance, student details can be retrieved and a Student instance is returned:

Student student = getStudent(scanner);


Joining strings with delimiter

Since Java 8, you can use String.join(CharSequence, CharSequence) to easily join strings:

private static String toFileOutput(Student student) {
    return String.join(DELIMITER,
            Integer.toString(student.getID()),
            student.getFirstName(),
            student.getLastName(),
            student.getGpa());
}


Determining the file output

Instead of reassigning the writers to a writer, you should be using them directly:

Student student = getStudent(scanner);
while (student.getID() != QUIT) {
    String output = toFileOutput(student);
    if (student.isHonors()) {
        honorsWriter.println(output);
    } else if (student.isGoodStanding()) {
        goodStandingWriter.println(output);
    } else if (student.isProbation()) {
        probationWriter.println(output);
    }
    student = getStudent(scanner);
}


for vs while

Another way of looping through each Student input is to use a for loop, this approach is sometimes (often? always?) recommended to reduce the scope of the student instance:

for (Student student = getStudent(scanner);
     student.getID() != QUIT;
     student = getStudent(scanner)) {
    // ...
}


Shortening the process

Alternatively, you can consider Files.write(Path, Iterable, OpenOption) to expressively write (pun unintended) a line of output, the only thing to be aware of is that you will need to turn your single String into an Iterable, e.g. using Collections.singleton(T).

Putting it altogether:

public static void main() {
    try (Scanner scanner = new Scanner(System.in)) {
        for (Student student = getStudent(scanner);
             student.getID() != QUIT;
             student = getStudent(scanner)) {
            Set output = Collections.singleton(toFileOutput(student));
            if (student.isHonors()) {
                Files.write(HONORS, output, APPEND);
            } else if (student.isGoodStanding()) {
                Files.write(GOOD_STANDING, output, APPEND);
            } else if (student.isProbation()) {
                Files.write(PROBATION, output, APPEND);
            }
        }
    } catch (IOException e) {
        throw new RuntimeException(e);
    }
}

Code Snippets

private static final Path HONORS = Paths.get("Honors.txt");
private static final Path GOOD_STANDING = Paths.get("GoodStanding.txt");
private static final Path PROBATION = Paths.get("Probation.txt");
private static final String DELIMITER = ",";
private static final int QUIT = 999;

public static void main(String[] args) {
    try (PrintWriter honorsWriter = new PrintWriter(HONORS.toFile());
         PrintWriter goodStandingWriter = new PrintWriter(GOOD_STANDING.toFile());
         PrintWriter probationWriter = new PrintWriter(PROBATION.toFile());
         Scanner scanner = new Scanner(System.in)
    ) {
        // ...
    } catch (IOException e) {
        throw new RuntimeException(e);
    }
}
Student student = getStudent(scanner);
private static String toFileOutput(Student student) {
    return String.join(DELIMITER,
            Integer.toString(student.getID()),
            student.getFirstName(),
            student.getLastName(),
            student.getGpa());
}
Student student = getStudent(scanner);
while (student.getID() != QUIT) {
    String output = toFileOutput(student);
    if (student.isHonors()) {
        honorsWriter.println(output);
    } else if (student.isGoodStanding()) {
        goodStandingWriter.println(output);
    } else if (student.isProbation()) {
        probationWriter.println(output);
    }
    student = getStudent(scanner);
}
for (Student student = getStudent(scanner);
     student.getID() != QUIT;
     student = getStudent(scanner)) {
    // ...
}

Context

StackExchange Code Review Q#154266, answer score: 5

Revisions (0)

No revisions yet.