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."
Additionally, 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.
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.