patternjavaMinor
The most efficient way to merge two lists in Java
Viewed 0 times
themergeefficientwayjavatwolistsmost
Problem
I am looking for a way to merge two files with a list of distinct words, one word per line. I have to create a new txt file that would contain all the words of the first list and all the words from the second list. I don't have any other specifications. The order of words in the result doesn't matter.
public class testMain {
public static void main(String[] args) {
File f1=new File("words.txt");
File f2=new File("words1.txt");
HashSet hash1=new HashSet();
HashSet hash2=new HashSet();
try{
Scanner s=new Scanner(f1);
while(s.hasNextLine()){
hash1.add(s.nextLine());
}
s=new Scanner(f2);
while(s.hasNextLine()){
hash2.add(s.nextLine());
}
}
catch(FileNotFoundException e){}
hash1.addAll(hash2);
Object[]array =hash1.toArray();
File newFile=new File("mixOfLists.txt");
try{
PrintWriter writer=new PrintWriter(newFile);
for(int i=0; i<array.length; i++){
writer.println(array[i]);
}
writer.close();
}
catch(FileNotFoundException e){ System.out.print("No Such File");}
System.out.print("Done!");
}
}Solution
Exception Handling
Your handling is not great.... this is a sign of poor forward planning:
And this is a sign of something almost as bad:
The first time I read that, I got confused and thought the
Style
1-liner blocks are seemingly convenient but in the long term can have negative impacts on maintainability. You have a lot of them, and they make reading your code hard.
Your code is also suffocating due to lack of whitespace. You need to put spaces around operators to help the code to breath.... yeah, that sounds alarmist, but it really helps.
should be:
Resources
You should use try-with-resources for your IO sources and sinks. As things stand at the moment, you don't close the readers properly.
Algorithm
You're reading both files in to their own sets, and then merging the sets, and then outputting the result.
A better solution would be to use the boolean return value from the
The above code can be used for both the input files, and only writes out the word if the word has not been seen before.
This way you have only one set, and you do the merge at the same time as the reading.
Also, you should be using Java 8 streams..... hmmm... that would be nice.
Your handling is not great.... this is a sign of poor forward planning:
catch(FileNotFoundException e){}And this is a sign of something almost as bad:
catch(FileNotFoundException e){ System.out.print("No Such File");}
System.out.print("Done!");
}The first time I read that, I got confused and thought the
"Done!" println was part of the exception handling. You need to work on the indentation. Also, just printing "No such file" is not a very helpful exception handling.Style
1-liner blocks are seemingly convenient but in the long term can have negative impacts on maintainability. You have a lot of them, and they make reading your code hard.
Your code is also suffocating due to lack of whitespace. You need to put spaces around operators to help the code to breath.... yeah, that sounds alarmist, but it really helps.
File newFile=new File("mixOfLists.txt");
try{
PrintWriter writer=new PrintWriter(newFile);
for(int i=0; i<array.length; i++){
writer.println(array[i]);
}should be:
File newFile = new File("mixOfLists.txt");
try{
PrintWriter writer = new PrintWriter(newFile);
for(int i = 0; i < array.length; i++) {
writer.println(array[i]);
}
....Resources
You should use try-with-resources for your IO sources and sinks. As things stand at the moment, you don't close the readers properly.
Algorithm
You're reading both files in to their own sets, and then merging the sets, and then outputting the result.
A better solution would be to use the boolean return value from the
add(...) method to determine whether the word has been seen before... consider:while(s.hasNextLine()){
String line = s.nextLine();
if(hash1.add(line)) {
writer.println(line);
}
}The above code can be used for both the input files, and only writes out the word if the word has not been seen before.
This way you have only one set, and you do the merge at the same time as the reading.
Also, you should be using Java 8 streams..... hmmm... that would be nice.
Code Snippets
catch(FileNotFoundException e){}catch(FileNotFoundException e){ System.out.print("No Such File");}
System.out.print("Done!");
}File newFile=new File("mixOfLists.txt");
try{
PrintWriter writer=new PrintWriter(newFile);
for(int i=0; i<array.length; i++){
writer.println(array[i]);
}File newFile = new File("mixOfLists.txt");
try{
PrintWriter writer = new PrintWriter(newFile);
for(int i = 0; i < array.length; i++) {
writer.println(array[i]);
}
....while(s.hasNextLine()){
String line = s.nextLine();
if(hash1.add(line)) {
writer.println(line);
}
}Context
StackExchange Code Review Q#92908, answer score: 6
Revisions (0)
No revisions yet.