3
\$\begingroup\$

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)
 {
 //get Student First Name
 System.out.print("Enter Student's First Name: ");
 student.setFirstName(input.next());
 //get Student Last Name
 System.out.print("Enter Student's Last Name: ");
 student.setLastName(input.next());
 //get Student GPA
 System.out.print("Enter Student's Grade Point Average: ");
 student.setgpa(input.nextDouble());
 //if gpa is 3.6 or> write to Honors Student
 if (student.Honors())
 writer=writer1;
 //if gpa is >=2.0 and >3.5 write to Good Standing Student
 else if (student.Good_Standing())
 writer=writer2;
 //if gpa is <2.0 write to Probation Student
 else if (student.Probation())
 writer=writer3;
 //out of range 
 else
 continue;
 //build the record
 recOut = student.getID() + delimiter
 + student.getFirstName() + delimiter
 + student.getLastName() + delimiter 
 + student.getgpa();
 //write the record
 writer.write(recOut, 0, recOut.length());
 writer.newLine();
 //subsequent read for indefinite repetition
 System.out.print("Enter a Student's ID number or 999 to quit: ");
 student.setID(input.nextInt());
 }
 //close all files
 writer1.close();
 writer2.close();
 writer3.close(); 
 }
 catch(Exception e)
 { 
 System.out.println("<<An exception has occurred.>> ");
 System.out.println(e.getMessage()); 
 } 
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 2, 2017 at 15:09
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

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<String> 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);
 }
}
answered Feb 2, 2017 at 16:58
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Do we need to provide the OpenOption.APPEND parameter for Files.write(...) ? \$\endgroup\$ Commented Feb 2, 2017 at 20:43
  • 1
    \$\begingroup\$ @oopexpert good catch! Have updated my answer. \$\endgroup\$ Commented Feb 2, 2017 at 23:47
1
\$\begingroup\$
  1. You should not distorte semantics. student.getID() != QUIT is semantically strange. My suggestion is to check for QUIT BEFORE creating a Student
  2. You should not reuse of the Student-object for different Students. Technically this is ok but it's not object-oriented and not semantically correct either
  3. Try-catch with resources is the proper approach handling File I/O Errors
  4. I do not suggest to hold the files open during long user input. My way would be to have the files open when writing is neccessary and closed during user input.
answered Feb 2, 2017 at 20:42
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.