Override because you can
-
So I've been assigned the maintenance on a hand-rolled "log parser" that's just bristling with odd decisions and general buggery. The first odd decision was that we should parse a log file to put data from the log file into a database. The following odd decision came somewhere further down the line, but managed to make it past code review and into the first commit of this file, verbatim:
@Override protected String readLine(String quitBeforeTok, BufferedReader reader, BufferedWriter echoWriter) throws Exception { String line = super.readLine(quitBeforeTok, reader, echoWriter); if (line == null){ return line; } return line; }
-
@DemonWasp said:
So I've been assigned the maintenance on a hand-rolled "log parser" that's just bristling with odd decisions and general buggery. The first odd decision was that we should parse a log file to put data from the log file into a database. The following odd decision came somewhere further down the line, but managed to make it past code review and into the first commit of this file, verbatim:
@Override protected String readLine(String quitBeforeTok, BufferedReader reader, BufferedWriter echoWriter) throws Exception { String line = super.readLine(quitBeforeTok, reader, echoWriter); if (line == null){ return line; } return line; }
There is so much more wrong with that piece of code than what you think at first glance. Truly a work of art WTF, well done. Now, please, kill it with fire.
-
@DemonWasp said:
So I've been assigned the maintenance on a hand-rolled "log parser" that's just bristling with odd decisions and general buggery. The first odd decision was that we should parse a log file to put data from the log file into a database. The following odd decision came somewhere further down the line, but managed to make it past code review and into the first commit of this file, verbatim:
@Override protected String readLine(String quitBeforeTok, BufferedReader reader, BufferedWriter echoWriter) throws Exception { String line = super.readLine(quitBeforeTok, reader, echoWriter); if (line == null){ return line; } return line; }
TRWTF is multiple return statements. </sarcasm>
But seriously, I would hope that the original developer had intended on adding more specific logic at some point and was just stubbing it out as a placeholder. But, I generally am too generous with my BotD giving. :)
-
@pbean said:
There is so much more wrong with that piece of code than what you think at first glance. Truly a work of art WTF, well done. Now, please, kill it with fire.
Indeed, where does one even begin? Wait right here: WHY DOES READING REQUIRE A WRITER?? Also, BufferedReader already has a method called readLine(). And it throws a top-level Exception? Sure.
Presumably "Tok" stands not for "Token" but "Toking". Unfortunately, the developer did not quitBeforeTok.
-
@dohpaz42 said:
But seriously, I would hope that the original developer had intended on adding more specific logic at some point and was just stubbing it out as a placeholder.
I would agree if it never got committed!
-
@hoodaticus said:
I would agree if it never got committed!
Sadly, it did. Twenty commits later, the method is entirely unmodified (until I removed it a moment ago). TRWTF, of course, is found in the first sentence of the OP.
-
PML.
Ve haf vays of making you Tok.Best bits are:
- It returns "line" whether it's null or not.
- You would expect "line" not to be returned as null from super.readLine, as that would just be wrong.
- Oh FMR, I've finally worked out that there is no point in this method at all. The penny's just dropped.
-
@dohpaz42 said:
But seriously, I would hope that the original developer had intended on adding more specific logic at some point and was just stubbing it out as a placeholder.
"Seriously"? "Seriously"?!?!
If any of my coworkers were to try to rationalize an abomination like the one above with reasoning like that, I'd smack them well across the room with the biggest cluebat I could find.
If you want to add more logic, then do it now or never! If you don't - leave things f-ing alone!
-
@hoodaticus said:
I would agree if it never got committed!
Just yesterday I was fixing a bug. It turned out that the code was trying to register with an object that was not there (and not checking null), because the particular device didn't have that particular sensor. So I checked why the code needs those data. Well, two calls later, in a different thread, it logged them (only if debugging was on) and forgot them. Fix was to just delete 170 lines.
I guess the WTF is that I am not even surprised.
-
@Anonymouse said:
"Seriously"? "Seriously"?!?!
If any of my coworkers were to try to rationalize an abomination like the one above with reasoning like that, I'd smack them well across the room with the biggest cluebat I could find.
If you want to add more logic, then do it now or never! If you don't - leave things f-ing alone!
Down boy! Breath.
-
@Anonymouse said:
If you want to add more logic, then do it now or never! If you don't - leave things f-ing alone!
Ah, the YNGNI principle.
-
@dohpaz42 said:
But seriously, I would hope that the original developer had intended on adding more specific logic at some point and was just stubbing it out as a placeholder. But, I generally am too generous with my BotD giving. :)
In that case, they failed by not adding a comment explaining their intentions.
-
@lolwtf said:
@dohpaz42 said:
But seriously, I would hope that the original developer had intended on adding more specific logic at some point and was just stubbing it out as a placeholder. But, I generally am too generous with my BotD giving. :)
In that case, they failed by not adding a comment explaining their intentions.Comments: making bad code good since 1951.