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());
}
}
}
2 Answers 2
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);
}
}
-
1\$\begingroup\$ Do we need to provide the OpenOption.APPEND parameter for Files.write(...) ? \$\endgroup\$oopexpert– oopexpert2017年02月02日 20:43:30 +00:00Commented Feb 2, 2017 at 20:43
-
1\$\begingroup\$ @oopexpert good catch! Have updated my answer. \$\endgroup\$h.j.k.– h.j.k.2017年02月02日 23:47:16 +00:00Commented Feb 2, 2017 at 23:47
- You should not distorte semantics. student.getID() != QUIT is semantically strange. My suggestion is to check for QUIT BEFORE creating a Student
- 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
- Try-catch with resources is the proper approach handling File I/O Errors
- 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.