3
\$\begingroup\$

This is an iterative review: The Original Question | Follow up 1

Differences to Follow up 1

This iteration tries to take the previous review to heart and see how it turns out. It sacrifices the flexibility to easily exchange sourceReader/sinkWriter classes that may read/write from different sources/sinks and specify that they read/write as BufferedReader/PrintWriter from/to files specified in the arguments in order to make the program less bulky. I'm unsure how to evaluate this change because I see reasons pro flexibility/bulky program as well as contra flexibility/bulky program. For the magnitude of this program and its actual task, I guess it might have been overkill.


Here the relevant sections of the previous questions:

The task itself is to go through a large .txt file that contains lines with ">" as their first character and to remove all spaces in these lines. I am using this program to modify files in FASTA format, a format often used in biology.

Algorithm

  1. Read next line from input file and store in "line"
  2. If "line" contains ">" as first character, remove all spaces in "line"
  3. Print "line" to output file
  4. If next line is not null, go back to Step 1

Changes to Argument Handling

In the process I also updated the argument handling process with my custom-written classes. Argument handling is still done by an ArgumentHandler, custom for every program, and an ArgumentCollectionImplementation class that implements the interface ArgumentCollection.

The ArgumentCollectionImplementation contains a List of all flags possible for this program upon instantiation (including a default String to print if -h is called) and gets the arguments of these flags added to it during argument parsing. It contains methods to receive and change one or all arguments of a given flag as well as methods to add and test arguments for int/double.

Setting default values of arguments, throwing errors if mandatory flags don't have an argument and similar tasks are all handled by the ArgumentHandler, implementing interface Configuration (of which I see very little use outside of this program) that provides the necessary methods getSinkFile()and getSourceFile() for the Client. Since the ArgumentHandler is program specific, it makes only sense to include it as private static class of the Client RemoveSpacesClient.


Client - including ArgumentHandler

public class RemoveSpacesClient {
    private static class RemoveSpacesArgumentHandler implements Configuration {
        private ArgumentCollection arguments;

        RemoveSpacesArgumentHandler(String[] args) {
            /*- Define all allowed Flags in the ArgumentCollection and add text for help flag:
             * -i: input flag
             * -o: output flag
             * -h: help flag*/
            String[] argumentList = { "-i", "-o", "-h", "Text to display if -h is called" };
            arguments = new ArgumentCollectionImplementation1(argumentList);

            /*- Parse arguments and add them to their associated flags in the ArgumentCollection*/
            arguments.parseArguments(args);

            /* Test mandatory flag -i for arguments */
            if (!arguments.flagHasArguments("-i")) {
                throw new RuntimeException("Mandatory flag -i was not specified");
            }

            /* Add a default argument to flag "-o" if it has no argument */
            if (!arguments.flagHasArguments("-o")) {
                arguments.addFlagArgument("-o", 0, arguments.getFlagStringArgument("-i") + ".nospace.txt");
            }
        }

        public File getSourceFile() {
            return new File(arguments.getFlagStringArgument("-i"));
        }

        public File getSinkFile() {
            return new File(arguments.getFlagStringArgument("-o"));
        }

        public void printArguments() {
            arguments.printArguments();
        }
    }

    private static void printProgramProgress(int i) {
        if (i % 1000000 == 0) {
            System.out.println(i / 1000000 + " * 10^6 lines written.");
        }
    }

    public static void main(String[] args) throws IOException {
        Configuration arguments = new RemoveSpacesArgumentHandler(args);

        System.out.println("Starting Program with the following arguments: ");
        arguments.printArguments();

        BufferedReader sourceReader = new BufferedReader(new FileReader(arguments.getSourceFile()));
        PrintWriter sinkWriter = new PrintWriter(arguments.getSinkFile());

        /*
         * Write every line from source-file to sink-file. If the line is a
         * sequence name (first character is ">"), remove all spaces in it
         * before writing.
         */
        int i = 0;
        for (String line = sourceReader.readLine(); line != null; line = sourceReader.readLine()) {
            if (line.charAt(0) == '>') {
                line = line.replace(" ", "");
            }
            sinkWriter.println(line);
            printProgramProgress(++i);
        }
        sourceReader.close();
        sinkWriter.close();
        System.out.println("Finished!");
    }
}

In a seperate class "CustomInterfaces" I have also Configuration

public interface Configuration {
    /**
     * Returns the name and path of a file to read from in form of a String.
     */
    File getSourceFile();

    /**
     * Returns the name and path of a file to write to in form of a String.
     */
    File getSinkFile();

    /** Prints all arguments */
    void printArguments();
}

Requests

  1. Am I correct in my assessment that a Configuration interface here is somewhat unnecessary, seeing as I think I would hardly be able to reuse the interface.

  2. The Client code throws 4 places where Exceptions can occur:

    1. When sourceReader is instantiated (FileNotFoundException),
    2. when sinkWriter is instantiated (FileNotFoundException),
    3. when sourceReader reads a line from source (IOException),
    4. when sourceReader is closed (IOException).

I came to the conclusion that there is not a lot the Client can do in case any of these occur, baring terminal-User-interaction that force the user to re-enter incorrect arguments until the input no longer causes exceptions. As such I let main throw an IOException. Was that wrong?

\$\endgroup\$
2
\$\begingroup\$

In the following code :

    BufferedReader sourceReader = new BufferedReader(new FileReader(arguments.getSourceFile()));
    PrintWriter sinkWriter = new PrintWriter(arguments.getSinkFile());

    /*
     * Write every line from source-file to sink-file. If the line is a
     * sequence name (first character is ">"), remove all spaces in it
     * before writing.
     */
    int i = 0;
    for (String line = sourceReader.readLine(); line != null; line = sourceReader.readLine()) {
        if (line.charAt(0) == '>') {
            line = line.replace(" ", "");
        }
        sinkWriter.println(line);
        printProgramProgress(++i);
    }
    sourceReader.close();
    sinkWriter.close();
    System.out.println("Finished!");

if an exception occurs at any point, files may not be properly closed.

I won't show the java 6 idiom for managing this case (because it's ugly and if you are still using java 6 you really should upgrade your version) but if you are using java 7, you should use try-with-ressources to avoid all the unnecessary closes.

This is how it looks like :

try (BufferedReader sourceReader = new BufferedReader(new FileReader("yolo"))) {
    // do some stuff here
} // no need to close sourceReader

If you are using java 8+, you should take a look the new static methods in the Files class (nio package) and how to manipulate Stream objects.

About your Configuration interface, yeah... it's better removed, because it is so coupled to the rest of your code you will probably never have another implementation besides RemoveSpacesArgumentHandler.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.