- 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
The frist thing I would do is change the contents of the exception trap to a throws and see what happens ...
Admin
If you have a solution to it then I'd love to see how you transitively compare "Foo", "12" and "13". No hurry, I'll wait.
And as for "p1.CompareTo(p2)". That's a lexicographic comparison, not a numerical one.
Admin
First of all. If a.compareTo(b) == 0 it doesn't mean that a.equals(b). And: If the submitter is changing the current implementation to p1.compareTo(p2), "11" will be seen as more important than "2".
Admin
I love it when the reason give for not fixing bugs is "because its in a critical part of the code". Surely this is paramount to negligence and I would like it if more customers complained about such negligence. Might start to get rid of the idiot's in the IT industry then and start having some professionalism around here.
Admin
You could argue that fixing it might introduce other bugs. A known, not critical bug is better than an unknown, possibly critical one
Admin
MacOS / iOS have a string comparison method with a multitude of options that will among other things sort numbers correctly when they occur inside.a string. So Foo < Priority 1 < priority 9 < priority 20 < 12 < 13 < 101.
Admin
... except that you really want priority 9 < 12 < 13 < priority 20 < 101.
Admin
If Communication.getPriority() is a method that returns a String and not a number, then you should start analyzing what is wrong instead of programming a garbage-in-garbage-out algorithm. Even comparing Strings, just to be on the safe side without exception handling, is wrong. If you compare priorites like "A", "B", "C" as strings, it would be sorted correctly, but priorities like "01", "10" or "A9", "A19", or "second", "fourth" not. Conversion to an integer should not be a job done outside the Communication class, but a method of the Communication object. So the first thing a programmer should do is to realize that it is impossible to program a comparison without further analysis of the data. He should not just implement something that is half-brewn, but something that works in all cases. If you print out a list of all possible values from the database then for example you find out that there are only the values "High", "Medium" and "Low" and constraints on the database enforcing only these 3 possible priorities.
But what if you are not allowed to change the Communication class to add a getPriorityAsInt() mehod and to make a sanity check in the constructor of the object for fail-fast behaviour? Well, then you should use composition (or inheritance): make your own clean communication object: define a new class that holds the original dirty communication object and implement fail-fast checks (in the constructor and whenever somebody wants to change the priority-String). Then only use your own communication objects in your whole program.
In all cases: The real WTF is believing "if it is not parseable, it is equal" instead letting the parse-error bubble up.
Admin
Well "Foo" clearly has top priority. Followed by "Bar". "13" comes after "12", unless its a Friday. In which case "13" is unlucky and has lower priority. Unless it is raining.
Admin
What this says to me is that Priority really ought to be an enum (at least if this were C++; I'm not so familiar with Java). Of course enums aren't immune to abuse, but correct usage of enums is easier to enforce than arbitrary strings.
Admin
Admin
The everything-is-a-string mentality is a sign that H1Bs are involved. The fear of never ever touching anything even if it's clearly broken is another. What astonishes me, from experience, is how never-ever-touch-anything and rework-rework-rework-forever can co-exist like they do.
Admin
Sounds like my colleagues requirements.
Admin
But what about the other big bug in this code? p1 and p2 are of the Integer class, not primitives of type int. So will p1 > p2 not compare the Objects (references) instead of the values?
Or is my 2000-ish knowledge of Java hopelessly outdated?
Admin
The answer is: It depends. Due to some caching magic, if the Integers p1 and p2 are between -127 and +127, the primitive comparators (< and >) will work properly. Otherwise, they should use p1.compareTo(p2).
Admin
There's a potential bug in there too. Although both p1 and p2 are created using Integer.parseInt (which returns an int), they are assigned to Integer variables. This means they will be autoboxed. Autoboxing uses cached values only for values between -128 and 127 (inclusive). That means that for any two equal values within that range, the autoboxing will use the same Integer object and p1 == p2 will be true. If both are 128, a non-cached, new Integer object will be returned. As a result, p1 != p2.
Fortunately, the trailing "return 0" will catch this case, but it clearly shows that the author of this piece of code has no idea what he's doing.
A better solution:
Admin
I would not use lexicographic parsing in case numeric one fails. That way, 9 < 10 < 10a < 8a < 9.
To get it number-safe, failsafe and transitive, the failure code has to be a lot more robust. One option I can think of (please point out if there are any mistakes) would be:
(If you repeat the same algorithm for the rest, you probably have an equivalent of the iOS sorting function)
Admin
The OS has a sting comparison method? This seems wrong.
Addendum 2019-01-07 16:22: "sting" s.b. "string"
Admin
It's not that hard to write.
Bilbo's sword > The wrestler > The thing bees do to hurt you > That godawful singer
Admin
I have a solution which leaves the non-critical bug intact, doesn't introduce new (possibly critical) bugs -and- improves performance! Your users will thank you: public int compare(Communication o1, Communication o2) { return Random.next(2) - 1; }
Admin
I think you mean Random.nextInt(3) - 1. Random.next(2) returns a 2-bit number (0, 1, 2 or 3). Although, returning a 2 (3 - 1) is valid, it just adds more weight to returning a positive number.
Admin
Sort of. If this were 2000-era Java, it would be a compile error. It works (for a loose definition of "works") because of autoboxing / autounboxing that appeared in Java 5 - automatic conversion between primitive types and their object counterparts. So just as the primitive types returned by parseInt were automatically converted to Integer objects, the Integer objects are being automatically converted to primitive ints in the comparisons... so it does actually work as intended.
However, it's not really good practice to write code that's constantly converting between the two forms, since it does incur some small overhead (particularly when converting to objects). Really, p1 and p2 should have been declared primitive, since there's no possibility of them being null.
Admin
String comparison in the OS means people don't roll their own, incorrect one. Or use one posted to GitHub by some rank amateurs. File names are sorted intelligently. Like version 10 to 19 of a document not between version 1 and version 2. The whole range of Unicode handled correctly. PLUS localised versions. Would you know how to sort in Swedish? You don't have to.
Admin
The main point in my post before is that you should analyze the data before coding. You spend a lot of time developing a string sorting algorithm with prefixes, numbers and suffixes, but it is all for nothing if it turns out that the strings contain only the three possible values "Low", "Medium" and "High".
But even if they contain numbers, how do you know if "1" is higher priority than "2"? For example "DEFCON 1" has higher priority than "DEFCON 5".
You just make assumptions which are not better than the assumption of the original programmer, all strings contain parseable numbers. This assumption obviously failed.
There are some problems that cannot be solved without prior investigation, and as a responsible programmer you should reject programming a "lucky game" that can destroy the reputation of your company.
Admin
No, you don't want "20" < "priority 21", you want the sender to give you the correct type.
Change the signature to only accept ints. Not Integers, not strings, just ints.
Fix the compiler errors, because the compiler will tell you everywhere you're using a string.
Then you're done and it actually works.
Admin
No. The
Integerclass does numeric comparison. (Really! Look it up if you don't believe me.)Admin
Agree.
But I'll also raise support for what Code Refactorer said. The fact a dev can concoct an algorithm which transitively compares "foo", 12, and "13" is still wrong if it doesn't compare the same way as the humans who decided those various value should be comparable.
Our goal as devs is to model a very messy real world in our oh-so-precise and oh-so-delicate code. Ideally along the way we teach the business to be more disciplined and less PHB-driven and messy. But we're stuck with the underlying reality that "correct" is totally in the eye of the customer-user and has nothing to do with lexicography, Unicode, or integer sorting.
If they're crazy enough to declare "foo" to be a valid priority, they're also the only ones who can tell us whether that sorts after "chocolate" and before "banana" or between 17 & 18. Or between 42 and Unicode U+1F4A9 💩.
Admin
The worst part is that the code contains a hidden bug
When ran it gives the following:
compare(1, "a") == 1 compare("a", 1) == 0
This is because both strings are converted to numbers in the same try-catch block
The worst part of these kind of bugs is that it may randomly break at a Java update, as Java updates only test correctness based on what the spec for compare say
Admin
Because it's entirely obvious that 😢 should come after 😄 but before 😻