The code looks good. I only have small things to point out:
isEmpty check
DefaultLogReader has a Set of EntryParser, and it's final. You even check if it's not null via Objects.requireNotNull ... but you don't check if it actually contains something.
For DefaultLogReader, that means that if you supply an empty set of EntryParsers, you'll get an NotReadableException the first time you try to read something. I'm not sure that's something you want. Given that you have made it an exception rather than a member attribute, I'm inclined to say that you should add checks to see if collections passed to you actually contain something.
Data Structures
private final List<E> peekedElements = new ArrayList<>();
@Override
public boolean hasNext() {
Optional<E> peekElement = peek();
return peekElement.isPresent();
}
@Override
public E next() {
if (!peekedElements.isEmpty()) {
return peekedElements.remove(0);
}
return iterator.next();
}
@Override
public boolean nextMatches(final Predicate<? super E> condition) {
Objects.requireNonNull(condition, "condition");
Optional<E> peekElement = peek();
return (peekElement.isPresent() && condition.test(peekElement.get()));
}
/**
* Returns an optional containing the next element, or an empty optional if there is none.
*
* @return The optional containing the next element, or the empty optional if there is none.
*/
private Optional<E> peek() {
if (!peekedElements.isEmpty()) {
return Optional.ofNullable(peekedElements.get(0));
}
if (!iterator.hasNext()) {
return Optional.empty();
}
E element = iterator.next();
peekedElements.add(element);
return Optional.ofNullable(element);
}
Let's see...
You add to the end and remove from the beginning. No other interactions occur with the List.
Why did you go with a List? A Queue is a lot better for this (it's designed for this!).
Unneeded member variable
DefaultLogReader has linesInMemory. This is not needed; it can be a final local in readNextEntry.
Class name / type mismatch
You have FileLogReader that takes a BufferedReader. Isn't this a BufferedReaderLogReader? How do you know it's a file? I bet I could jury rig stdin into your FileLogReader. I suggest getting an InputStream instead and making it a StreamLogReader or perhaps even taking a File or a Path... but that might not be what you want. Alter the name appropriately after making a decision.
Constants
while (!bufferedReaderIterator.hasNext()) {
Thread.sleep(100);
}
Perhaps you want to make this somewhat configurable? I'd put in a constant POLL_INTERVAL or something.
Exceptions
You make use of Objects.requireNotNull in numerous places. Just providing the variable names is helpful, but perhaps a proper message "entryParsers are null for [class]"?
Parameter order
You've purposefully made each subclass of DefaultLogReader have their first argument be the most important one... which happens to be the iterator provider. So why do you then have entryParsers as first argument for DefaultLogReader? You're messing with the order of arguments and making implementers making do a double check (hang on, it's reversed?). It's a small point, but I don't have any bigger ones.
But part of a public API is the documentation that goes along with it. So lets focus on that.
Redundant documentation
/**
* Constructs a new DefaultLogReader instance.
*
* @param entryParsers The set of entry parsers
* @param readIterator The iterator used to read lines from the log source
* @param filterPredicate The predicate to use to filter the lines read from the log source
* @throws java.lang.NullPointerException If entryParsers, readIterator or filterPredicate is null.
*/
protected DefaultLogReader(final Set<? extends EntryParser> entryParsers, final Iterator<String> readIterator, final Predicate<? super String> filterPredicate) {
Objects.requireNonNull(filterPredicate, "filterPredicate");
Objects.requireNonNull(readIterator, "readIterator");
this.entryParsers = Objects.requireNonNull(entryParsers, "entryParsers");
Iterator<String> filteredIterator = IteratorUtils.filteredIterator(readIterator, filterPredicate);
this.matchingIterator = MatchingIterator.fromIterator(filteredIterator);
}
What can we learn from the documentation which is not learnable from the code?
Well, suppose the implementation went missing.
/**
* Constructs a new DefaultLogReader instance.
*
* @param entryParsers The set of entry parsers
* @param readIterator The iterator used to read lines from the log source
* @param filterPredicate The predicate to use to filter the lines read from the log source
* @throws java.lang.NullPointerException If entryParsers, readIterator or filterPredicate is null.
*/
protected DefaultLogReader(final Set<? extends EntryParser> entryParsers, final Iterator<String> readIterator, final Predicate<? super String> filterPredicate) {
//whoops
}
Information provided by documentation bolded.
This method creates a new DefaultLogReader. Well yeah, that's what constructors do.
entryParsers is a set of EntryParsers. That's what the code says, thanks.
readIterator is the iterator used to read lines from the log source. Ah, I see.
filterPredicate is the predicate to use to filter the lines read from the log source. Hmm.
Oh, and it will throw a NullPointerException if you give null for any of them.
You should strive to provide meaningful documentation for every single thing documented - or leave it blank. In this case, the general constructor message and entryParsers do not provide meaningful information.
Interesting, for instance, in that constructor at the top there is that the entryParsers are not copied. If you later modify that set, it will have effects on the internal workings of the DefaultLogReader. Oops.
For the method description - you have two constructors! Why is this one different? Consider writing something like "Constructs a new DefaultLogReader instance with line filtering."
Javadoc Links
/**
* Used to parse lines from a log source.
*
* It has the option to read more lines from the log source via a line reader if deemed necessary.
*
* @author Frank van Heeswijk
*/
public interface EntryParser
It has the option to read more lines from the log source via a line reader if deemed necessary.
I'd suggest placing a @link here. http://stackoverflow.com/questions/10097199/javadoc-see-or-link
Like that, you emphasize that it can use a LineReader rather than some object.
Double space
/**
* Exception to indicate that a log entry is not readable.
*
* @author Frank van Heeswijk
*/
public class NotReadableException extends Exception {
Exception {
Check your auto-formatting settings. Auto-format before posting a question on CR/committing to a repository.