Try/Catch interfering with while loop

Question

So I'm trying to write a code that searches a txt file for a specific string, then prints all lines on which the string occurs.

The most straightforward way to do this seems to be running a Scanner and a LineNumberReader through the document, adding lines that fit the bill to the "found" string. However, whenever it stumbles across a line that doesn't contain the word, it throws a NullPointerException and kills the loop no matter what. Can anyone tell me what I'm doing wrong?

FileReader r = new FileReader(f);
    LineNumberReader l = new LineNumberReader(r);
    Scanner s = new Scanner(l);
    int i = 1;
    String found = "Instances of string found:\n";
    {
        while (s.hasNextLine()) {
            try {
                if (s.findInLine(keyword).isEmpty() == false) {
                    found = found + l.readLine() + "\n";
                    s.nextLine();
                } else {
                    s.nextLine();
                }
            } catch (NullPointerException e) {
                s.nextLine();
            }
        }
            display(found, "Match found!");

    }

You should be catching a runtime exception, especially a NPE. It pretty much translates to "You're doing it wrong". — Brian Roach, Mar 03 '14 at 02:13
Anyone who posts a question telling us how they get an exception but then not including the exception details in the question should be forced to hand write "I will include the exception in my post" 1000 times and attach that to the question (along with the exception...) — John3136, Mar 03 '14 at 02:15

score 1 · Accepted Answer · answered Mar 03 '14 at 02:15

Check the documentation of scanner: If no such pattern is detected in the input up to the next line separator, then null is returned and the scanner's position is unchanged.

You call s.findInLine(keyword).isEmpty() == false, if the word is not contained in findInLine(keyword) will be null, thus you'd be calling null.isEmpty(). There's your exception ;)

You don't have to check for isEmpty(), s.findInLine(keyword)!= null should be enough.

score 0 · Answer 2 · answered Mar 03 '14 at 02:35

If you're using a method that is documented as returning null in some cases, then you should assign the result of the method to a variable (if you're going to use it for something else) and use == or != to test it for null. It is very poor programming practice to use the result as a reference and then rely on try/catch on NullPointerException to see if it's null. For one thing, what if there's an unexpected null somewhere else in the try body? Now it will be catching the exception for the wrong reason. NullPointerException always indicates a program bug. It should never be part of the "normal" program logic.

As for why it "kills the loop": It shouldn't. Even though your use of try/catch is poor practice, it should still work the way I think you intended, and shouldn't kill the loop. I just tested something similar to your code, but without l.readLine(), and it seemed to behave. If you want the line number, it's l.getLineNumber(), not l.readLine(), which tries to read a line of text and could sabotage the Scanner.

Try/Catch interfering with while loop

2 Answers2