3

This argument has been going on for weeks:

Bot.getGuild().getMembers().stream()
        .filter((final Member m) -> m.getRoles().size() == 0)
        .filter((final Member m) -> !m.getUser().isBot())
        .forEach(JoinListener::assignDefaultRole);

vs.

Bot.getGuild().getMembers().stream()
        .filter(m -> m.getRoles().size() == 0)
        .filter(m -> !m.getUser().isBot())
        .forEach(JoinListener::assignDefaultRole);

Which is more readable?

Or, at what point is a liberal use of final excessive and unbeneficial, or even contraproductive? Consider that the latter snippet will draw extra attention if final is liberally used throughout the rest of the codebase.

And, how much shall be assumed about the type of a stream with respect to the originating method name?


Related questions:

  • This code is going to be much more readable without streams, in which case the use of final makes little difference. It would also highlights the strangeness of the assignDefaultRole and getGuild static methods. for (final Member m : Bot.getGuild().getMembers()) { if (m.getRoles().isEmpty() && !m.getUser().isBot()) { JoinListener.assignDefaultRole(m); } }. – Tom Hawtin - tackline Nov 18 '19 at 13:59
4

The final keyword is unnecessary. Not only from a technical perspective as final complex objects can still be manimulated (e.g. call clear on a final list) but also from a theoretical perspective.

In functional programming a function (in this case your lambda) is not supposed to manipulate structures. Execution is not happening by changing the state of structures as a side effect but by returning new structures that contain a new state. Therefore input parameters should never be manipulated. Moreover, for a filter, I would never expect that it manipulates a structure even if I have never heard of functional programming. Therefore the final is unnecessary.

For the type there can be some benefit by explicitly declaring it. However, instead of writing 'Member m -> ...' I would rather use a meaningful variable name and write 'member -> ...'. In this example this does not change much but sometimes you might be forced to cope with enterprise-style class names like 'AbstractOracleDatabaseAccessSerializer' which you cant change but definitely do not want to write all the time. (Please do not start a discussion on the names here. This is example is not helpful but I am sure it exists in real life ;))

So in your case I would probably do something like this

Bot.getGuild().getMembers().stream()
        .filter(member -> member.getRoles().size() == 0)
        .filter(member -> !member.getUser().isBot())
        .forEach(JoinListener::assignDefaultRole);

As a side note, when you have multiple filters in a row, I might make sense to combine them into a single filter function on the member class if the filter is needed more often or the filter condition is a concept of the Member class. This would enable you to use a member reference instead

Bot.getGuild().getMembers().stream()
        .filter(Member::checkXXX)
        .forEach(JoinListener::assignDefaultRole);
2

This is similar to use of final in method parameters

You can add final methods' parameters but it's most cases futile (except special cases)

Java always makes a copy of parameters before sending them to methods. This means the final doesn't mean any difference for the calling code. This only means that inside the method the variables can not be reassigned. (note that if you have a final object, you can still change the attributes of the object).

Also maybe you can change size() == 0 to isEmpty()

Returns true if this map contains no key-value mappings.

Bot.getGuild().getMembers().stream()
    .filter(m -> m.getRoles().isEmpty())
    .filter(m -> !m.getUser().isBot())
    .forEach(JoinListener::assignDefaultRole);

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.