- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
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.
Admin
Oh, my god! Your date comparisons!
Admin
You mean:
GenericEasyReaderVersion { return isBad == true ? true : false }?Admin
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).
Admin
Encapsulating stupid ideas in a single place is simply turning spaghetti code into ravioli.
Admin
This post remineded of a discussion which could be a WTF in itself https://www.realworldtech.com/forum/?threadid=202469&curpostid=202469
Admin
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.)
Admin
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.
Admin
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
Comparableinterface 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.
Admin
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.)
Admin
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)
Admin
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 withComparator.comparing(t -> t.transDate), like @JB suggested. Actually, you'd be better off encapsulating that field, at which point it'sComparator.comparing(Transaction::getDate)or something, but I digress.)Admin
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
Admin
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.
Admin
"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>
Admin
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.