CODEWITHSUNDEEP
javaexceptionrecursion
C. Ross
C. Ross

Reputation: 31848

Recursion in Error Handling?

I'm writing a file reader that returns an object and I'd like it to warn on parse errors and continue to the next record.

The code below is the obvious implementation of this, but involves recursing from inside the catch block. Is there any technical or stylistic reason not to do this?

public RecordType nextRecord() throws IOException{
    if (reader == null){
        throw new IllegalStateException("Reader closed.");
    }
    String line = reader.readLine();
    if (line == null){
        return null;
    }else{
        try {
            return parseRecord(line);
        }catch (ParseException pex){
            logger.warn("Record ignored due to parse error: " 
                + pex.getMessage());
            //Note the recursion here
            return nextRecord();
        }
    }
}

Upvotes: 2

Views: 4123

Answers (4)

Shaded
Shaded

Reputation: 17836

What it seems like to me is that whatever is calling your method is going to keep calling it until the method returns null.

I would probably follow the advice of the previous posters and use a loop, however I would look at whatever is calling the method (as it is probably already using a loop), have it skip the line by looking for an exception to be thrown.

Upvotes: 0

Paul Cager
Paul Cager

Reputation: 1920

Would it be cleaner to let the ParseException propagate back to the caller? The caller could then decide what to do about it.

Upvotes: 0

Peter Lawrey
Peter Lawrey

Reputation: 533530

I would prefer to use a loop. With recursion, you never know how deep you can safely go.

String line;
while((line = reader.readLine()) != null) {
    try {
        return parseRecord(line);
    }catch (ParseException pex){
        logger.warn("Record ignored due to parse error: " + pex);
    }
}
return null;

Upvotes: 3

NPE
NPE

Reputation: 500377

Why not replace the recursion with a loop:

public RecordType nextRecord() throws IOException {
    if (reader == null) {
        throw new IllegalStateException("Reader closed.");
    }
    for (;;) {
        String line = reader.readLine();
        if (line == null) {
            return null;
        } else {
            try {
                return parseRecord(line);
            } catch (ParseException pex) {
                logger.warn("Record ignored due to parse error: "
                        + pex.getMessage());
                // continue to the next record
            }
        }
    }
}

Stylistically, I find this preferable.

Upvotes: 0

Related Questions