The most efficient way to merge two lists in Java

Question

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 <String> hash1=new HashSet<String>();
        HashSet <String> hash2=new HashSet<String>();
        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!");
        }

}

Answer 1

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.

Answer 2

Mostly adding to rolfl's answer really...

1. Style

Indentation and spaces have already been mentioned here, so I won't quote, just re-iterate. The source code above is 36 lines long, not including the import statements that were omitted.

The line System.out.print("Done!"); should probably be done with println() as the formatting does not look great when the program throws errors.

2. Exceptions

The exceptions need work here. They are almost always one of the first things you take care of in a method that throws them, even before you write the code that could make it happen.

If exceptions aren't set up properly, or clear enough about what has happened then it can lead to unexpected outputs. For example, due to your empty block mentioned by rolfl, if either "words.txt" or "words1.txt" does not exist, the resulting merged file will not contain the missing content, and still cheerfully prints "Done!" at the end.

Also, a print message from the programmer is usually not enough to tell what has gone on, so e.getMessage() or e.getStackTrace() can be helpful if added.

3. Methods, Magic variables

Methods, dude! Always split tasks up into methods when possible, and in particular avoid a giant wall of text in the main() method. This helps to make the program flow less confusing and easier to debug.

Magic variables in programs can be fine in circumstances where they are declared as final, but I can't really think of a different example. There's usually always a better and more elegant solution.

http://en.wikipedia.org/wiki/Magic_string

If you allow users to enter filenames to merge through the program arguments, then you can do away with the magic input filenames altogether. There is only one magic string here, and it is declared at the TOP:

public class testMain {

    private static final String OUTPUT_FILENAME = "mixOfLists.txt";

    public static void main( String[] args ) {

        if(args.length < 2) {
            System.out.println("Must input at least two filenames to merge");
            System.exit(1);
        }

        mergeFiles(args, OUTPUT_FILENAME);
        System.out.println("Done!");
        System.exit(0);
    }

    public static void mergeFiles(String[] filesToMerge, 
                                                String outputFilename) { ...

Here, I have left the output filename as a constant to make the arguments easier to enter and process. The mergeFiles method still takes the input and ouput paramaters in case you want to use this method again.

4. Variable Names

Good variable names reveal what they are there for at first glance.

while(s.hasNextLine()){ is rather cryptic. Names like in, out, scanner are all fairly short and are effective.

5. Efficiency

I've found in Java that the variable that makes the most difference in performance is memory usage; if your memory usage is low, then your program is very likely to run fast.

• Multiple HashSets blow out the memory usage fairly quickly, and two aren't needed here (or actually at all) There is also an array that isn't needed.

• One of the main culprits under the hood here is Scanner, try rewriting with the BufferedReader and BufferedWriter classes: https://docs.oracle.com/javase/7/docs/api/java/io/BufferedReader.html http://docs.oracle.com/javase/7/docs/api/java/io/BufferedWriter.html

I tested a basic file with ~120 000 lines and got memory improvements of ~6x better when using the Buffered's

-Don't declare variables too long before you need to use them and they can go out of scope. The output filename is the only variable here that needs to be declared outside the try blocks, as all the file data can be read, written, closed and then forgotten about.

This is the implementation I came up with (very basic still). Not including the import statements it is 45 lines long, only 9 longer than the original :)

public class testMain {

    private static final String OUTPUT_FILENAME = "mixOfLists.txt";

    public static void main( String[] args ) {

        if(args.length < 2) {
            System.out.println("Must input at least two filenames to merge");
            System.exit(1);
        }

        mergeFiles(args, OUTPUT_FILENAME);
        System.out.println("Done!");
        System.exit(0);
    }

    private static void mergeFiles(String[] filesToMerge, 
                                                String outputFilename) {
        try {
            HashSet<String> mergeSet = new HashSet<>();
            BufferedWriter out = 
                    new BufferedWriter(new FileWriter(outputFilename));

            for(String filename : filesToMerge) {
                BufferedReader in = new BufferedReader(
                                            new FileReader(filename));
                String temp;
                while( (temp = in.readLine()) != null ) {
                    if(mergeSet.add(temp)) {
                        out.write(temp);
                    }
                }
                in.close();    
            }
            out.close();
        } catch ( FileNotFoundException e ) {
            System.err.println("File not found: "+ e.getStackTrace());
            System.exit(1);
        } catch ( IOException e ) {
            System.err.println("File error occurred: " + e.getStackTrace());
            System.exit(1);
        }
    }
}