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

The most efficient way to merge two lists in Java

Submitted by: @import:stackexchange-codereview··
0
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:

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.