• (nodebb)

    1, the code snippet has a mistake, it's comparing the date to another transaction itself, not its date. 2, using something like instanceof (or is in C#) to perform different logic depending on type is valid in some cases. Not all logic can be baked into a type; it's normal to have an "executor" type which executes operations on other types. However, this is not the case here, equality should not be implemented this way, it's often a core quality of the type and should be baked in.

  • Vilx- (unregistered)

    Oh, my god! Your date comparisons!

  • Hans (unregistered)

    You mean: GenericEasyReaderVersion { return isBad == true ? true : false }?

  • Anonymous') OR 1=1; DROP TABLE wtf; -- (unregistered)

    Not only is GenericComparitor bad for the obvious reasons, but it also allows you to attempt to compare different object types. Either it has O(N^2) different cases to handle all the different type combinations, or more likely it'll just throw something like a ClassCastException if you try to do compare(someA, someB).

  • my name is missing (unregistered)

    Encapsulating stupid ideas in a single place is simply turning spaghetti code into ravioli.

  • Anon (unregistered)

    This post remineded of a discussion which could be a WTF in itself https://www.realworldtech.com/forum/?threadid=202469&curpostid=202469

  • Foo AKA Fooo (unregistered)

    Sure, this implementation is terrible (readability, performance, everything), but the concern is valid, not having to remember which comparator to pass each time you want to sort something.

    I think that's a good argument to show that Java's way is not a good one, compared (no pun intended) to having operator overloading or some other way to bind a default comparator to a class (e.g. in C++, specializing std::less), so the users of the class don't have to worry about it. (You can still have sort etc. accept an optional comparator for cases where you want a different ordering, but the common case should be simple for the user.)

  • Vilx- (unregistered) in reply to Foo AKA Fooo

    I like C#'s solution - you can just have your classes implement IComparable, so that all the comparison logic is right there inside the class. I think this should also work in Java.

  • (author) in reply to Vilx-

    Java also has an equivalent, but the problem here is that you're baking it into the class implementation- difficult if it's not your class. Plus, while we may want to put objects in order for a variety of reasons, implementing a Comparable interface is defining that your class has an order. Like, we're baking ordinality into the definition of the class, which doesn't make sense for most classes.

    For the task of sorting, I think both C# and Java have a better solution- lambdas. And in Java, the lambda implicitly creates a class that implements the Comparator interface.

  • (nodebb) in reply to Remy Porter

    Plus, while we may want to put objects in order for a variety of reasons, implementing a Comparable interface is defining that your class has an order.

    The other reason for defining your own comparator is when you want to use a non-default ordering. Yes, strings have a default order, but sometimes you might want to use a case-insensitive ordering, or one that does special things with digit sequences, or one that works on soundex codes, or … well, there's an infinite space of bad ideas, but quite a few good ones too. Defining a whole sorter implementation for each of those would earn an article here, but defining a comparator is much easier.

    (There are other uses for comparators too, such as when you're doing binary searching.)

  • JB (unregistered)

    Arguably TRWTF is ever defining a Comparator in Java 8+, lambda or not. 99% of the time you just want to use some key function that maps your object to a comparable value, and you don't need a function that actually looks at two instances. You can write:

    List.of("foo", "bar", "foobar").sort(Comparator.comparing(String::length))

    and you can even chain these:

    Comparator.comparing(String::length).thenComparing(String::compareToIgnoreCase)

  • Naomi (unregistered) in reply to Foo AKA Fooo

    As an aside, you don't need to specify the parameter types, either - (t1, t2) -> t1.transDate.compareTo(t2.transDate) would work just fine. (Although you'd be better of with Comparator.comparing(t -> t.transDate), like @JB suggested. Actually, you'd be better off encapsulating that field, at which point it's Comparator.comparing(Transaction::getDate) or something, but I digress.)

  • (nodebb) in reply to my name is missing
    Encapsulating stupid ideas in a single place is simply turning spaghetti code into ravioli.

    But at least keeping it in one place gives you half a chance of successfully refactoring it without missing anything a'la copy-paste, or distributed (yet still nonsensical) comparison interfaces

  • Naomi (unregistered) in reply to caffiend

    Okay, there are several things wrong with this.

    First off, this class flagrantly violates the single responsibility principle, in that it's responsible for comparing who knows how many disparate types. It should be broken up. And secondly, it's type-unsafe. It's declared as implementing Comparator<Object>, but it won't sort a list containing arbitrary objects.

    It needs to be broken up.

  • jay (unregistered)

    "GenericEasyReaderVersion { return isBad == true ? true : false }"

    Just recently I saw a block of code that read: if <some condition> then isValid=true else if <other condition> then isValide=true else isValid=false end if end if if isValid then return true else return false end if

    I replaced the whole thing with return <some condition> orelse <other condition>

  • Jim (unregistered)

    The defendent claims a valid benefit for their code: that it makes it easy to know which Comparator to use. Any proposed alternative has to satisfy that requirement.

    How about having a single class with public static final comparators in it? If you know the name of that class Intellisense will get you to the right place. Got anything better?

    It's a shame Java can't do Generic Specializations.

Leave a comment on “A Fairly Generic Comparison to Make”

Log In or post as a guest

Replying to comment #531646:

« Return to Article