hazelcast / hazelcast-jet

Distributed Stream and Batch Processing
https://jet-start.sh
Other
1.1k stars 205 forks source link

Inconsistency in utility methods layout #1428

Open jerrinot opened 5 years ago

jerrinot commented 5 years ago

There is ComparatorEx which provides quite a few comparator implementations: naturalOrder(), reverseOrder(), etc.

Then there is a class FunctionEx. It provides identity(), but most of other useful function implementations are in in the class Functions. (from the same package).

Is there any system in this? It feels inconsistent.

viliam-durina commented 5 years ago

FunctionEx mimicks java.util.function.Function, which has identity(). Functions is our addition, there's no counterpart in j.u.f.

jerrinot commented 5 years ago

Why to have the separate Functions instead of adding our additions into FunctionEx ? As a user I don't care if a specified function is Jet addition or it's a part of Java SE. What if one of own additions end-ups in some later Java SE (inside j.u.f.Function) ? I don't see a good enough reason for this split. Do you? As a user I find it confusing - I am looking for some ready-to-use function, but I have to check two classes.

Is it the same for Comparators?

viliam-durina commented 5 years ago

Java's Collections, Arrays, Files, Paths, StandardCharsets are also collections of utility methods. But there are also many examples where the static factory methods are in the interface: Comparator, Stream, Function, Instant, DateTimeFormatter. So both patterns are common.

In Jet we also have Traversers, Sources, Sinks, Processors, AggregateOperations.

Your argument about Functions is valid, but ours is too: FooEx is a 1:1 clone of Foo which declares exception and extends Serializable. Our additions are separate, even though they are in the same package.

Also, the additions are pretty trivial. You're unlikely to discover them except by copy-pasting a code from somewhere, in which case you don't care much about the containing class.

cangencer commented 5 years ago

The main thing is that we have *Ex as an exact mirror of JDK classes without any additional methods. Perhaps it's not the strongest argument, but the idea was to keep those classes as close to the original as possible and the only thing to justify their existence is Serializability.

jerrinot commented 5 years ago

@viliam-durina I know about the duality in the Java SE platform itself. but itsnt it because of historical reasons? As the ability to add static method to interface was introduce relatively recently. Java 8 I think.

@cangencer yeah, the question is whether it's the best possible use experience. I am trying to think: as a user would I ever want to have it separated?

The strongest possible reason for the current separation is a potential clash when Java SE adds a new factory method under a name we already used for something else.

mtopolnik commented 5 years ago

While on the subject, my larger worry is that we have overloads in ComparatorEx (e.g., comparing(FunctionEx) and comparing(Function)) that are conflicting and are resolved in Java only through some pretty non-obvious rules. The same rules don't apply in Kotlin and you are forced to use a longer syntax to use them.