Small refactoring

[Rylern]

No description provided.

[petebankhead]

I've made a suggestion about how the filters/predicates are handled.

Please let me know what you think - also let me know if you'd rather merge this PR and discuss any changed later.

[petebankhead]

For consistency, I think the first thing passed to each method should be the same (e.g. here the text is passed second in one method, first for the other method).

I also think that should probably also be the LogMessage and not a String - since conceivably a filter might work on different fields from the message.

I also suggest the naming

• filterMessageByRegex
• filterMessageByText

isMessageFilteredByXXX also works

[petebankhead]

Actually, I want to modify my suggestion...

If you use

public static Predicate<LogMessage> createPredicateFromRegex(String regex) {
   ...
}

public static Predicate<LogMessage> createPredicateContains(String text) {
   ...
}

public static Predicate<LogMessage> createPredicateContainsIgnoreCase(String text) {
   ...
}

then the actual implementation methods can be private.

This has the advantage that the createPredicateContainsIgnoreCase method can lowercase the text once prior to passing it to the method that determines the predicate result, and not on every call - without requiring any external code to know to lowercase.

You can also then change the predicate implementations more easily (e.g. if you decide that we want predicates that work just on the log message, or ones that work on other fields of the message).

And you can change the default behavior easily (e.g. should a broken regex return true or false?)

Finally, this class seems a particularly good fit for some JUnit tests.

If you make these changes, the class name should be updated to reflect that, e.g. LogMessagePredicates or LogMessagePredicateUtils.

What do you think?

[Rylern]

Thanks, I implemented it

[petebankhead]

A couple more suggests (my code is untested, so may have typos or just not work...).

[petebankhead]

I'd rather avoid adding this to the public API unless it's really needed.
I'd expect that log messages should be immutable, and all the fields should be known from the start - so I think it should be possible to avoid adding this method (see comment below).

[petebankhead]

It'd be better to avoid generating a new LogMessage, and ensure that the text is made lowercase outside of the lambda. Suggested (untested!):

public static Predicate<LogMessage> createPredicateContainsIgnoreCase(String text) {
    if (text == null || text.isEmpty())
        return logMessage -> true;
    String textLower = text.toLowerCase();
    return logMessage -> logMessage.message().toLowerCase().contains(textLower);
}