Extensible Abstract Log Reader

My goal with the following code is to provide an extensible class that can be extended in order to read log files and transforms them into meaningful output.

I'd like to have comments about the code itself, the test code, the javadoc and the overall structure.

/**
 * Exception to indicate that there is no more input.
 *
 * @author Frank van Heeswijk
 */
public class NoMoreInputException extends Exception {
    private static final long serialVersionUID = -4640787627068619913L;

    public NoMoreInputException() {

    }

    public NoMoreInputException(final String message) {
        super(message);
    }

    public NoMoreInputException(final String message, final Throwable cause) {
        super(message, cause);
    }

    public NoMoreInputException(final Throwable cause) {
        super(cause);
    }
}

/**
 * Exception to indicate that a log entry is not readable.
 *
 * @author Frank van Heeswijk
 */
public class NotReadableException extends Exception  {
    private static final long serialVersionUID = -117259271357929934L;

    private final List lines = new ArrayList<>();

    /**
     * Constructs a new NotReadableException instance.
     *
     * @param lines The lines that were not readable
     */
    public NotReadableException(final List lines) {
        this.lines.addAll(lines);
    }

    /**
     * Returns the lines that were not readable.
     *
     * @return  The lines that were not readable.
     */
    public List getLines() {
        return new ArrayList<>(lines);
    }
}

```
/**
* Exception to indicate that a log entry is not parsable.
*
* @author Frank van Heeswijk
*/
class NotParsableException extends Exception {
private static final long serialVersionUID = 3147294996191143729L;

NotParsableException() {

}

NotParsableException(final String message) {
super(message);
}

NotParsableException(final String message, final Throwable cause) {
super(message, cause);
}

NotParsa

NoMoreInputException

"No more input" shouldn't be an exception, as it's part of the normal flow and not an unexpected condition. Use the same paradigm as Iterator and add hasNext() to LineReader and LogReader.

LineReader

I don't see any compelling reason not to prefer the Iterator interface.

EntryReader

Should this be named EntryParser? It doesn't read, it parses. Or really, it does both, which is messy.

AbstractLogLineReader

linesInMemory

I don't like this, just because it adds state to a class which could otherwise be stateless. There should be a way to refactor it out.

readEntry()

• Assumes that each entry is parseable by exactly one EntryReader. You may want to consider finishing the loop and throwing an error if multiple parsers are valid. The risk is using the wrong parser when multiples will work.

• Should be final, since you hopefully don't want subclasses to override this method.

General

As a user of this API subset, I guess I feel like there are a lot of moving pieces for me to worry about. I'd like to see one final class, LogReader, which either takes a BufferedReader containing a log file, a 'File' which it uses to build a Reader, or it figures out the file location for itself. It would have a default set of EntryParsers attached, and only if it makes sense give me a way to register more (directly on the object via constructor or addParser(), or some reflection/discovery process). It would have a method read() which returned an Iterator. Everything else would be behind the scenes, because I don't care about any of the rest of it.

The implementation would then need to build that Iterator. You might want something that can look at the log and pull lines until it has read a whole log entry, if possible, then pass that to the parser. You may be able to leverage BufferedReader#mark() and #reset(), depending on how performance-critical this is. Otherwise you're stuck with the "first line plus Reader" parameters to the parser, which is not my favorite but not always avoidable with multiline log files.

All of those exceptions should probably go away in favor of IOException, since you shouldn't be using them for business logic. It may be appropriate to throw a specific exception, but I'm not sold based on what you've shown. You may need a LogEntryNotParseable exception, I suppose. In that case you need one custom interface that's basically Iterator but next() would throw your exception. That lets you keep walking through the rest of the log entries if one can't be parsed.

StackExchange Code Review Q#77166, answer score: 8