- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- 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
You gotta love "Log Driven Programming".
Admin
If an Integer is not parsed is it still an Integer?
Admin
When exactly will this function return False?
Admin
I remember an article similar to this one not too long ago that involved logging where they could not find the source of the system slowdowns. It ultimately ended up being that they were logging everything for every method.
CAPTCHA: jumentum - when a Rabbi is on a role.
Admin
I'll bear this method in mind if I ever get paid by line of code.
Admin
IF an integer is parsed in the woods, and no one is around to hear it fail, does it raise an exception?
Captcha: Ingenium, that tangible liquid form of genius, now only $3.99 per 8oz. bottle!
Admin
I wonder if you could write a "log-file-compiler" taking the log file as input and producing a fully functional program?
Admin
When the Integer is NULL. I don't think NULL is a valid number. :)
Admin
Yup, you're right. This, however, wouldn't be my method of choice to test for NULLs.
Admin
That's one hell of an expensive null-checker.
Admin
Ok so logging each call to isValidNumber() is a wtf, but isn't having a paramater of type Integer also a wtf? Won't the call to the method fail if anything but an integer is passed?
Admin
Even worse, it will log that the thrown exception was a Number Format Exception, rather than the true Null Pointer Exception. This could make debugging problems interesting.
Admin
When the passed parameter is null
Admin
Admin
That's what happens when you pay per line for outsourcing!
Admin
Admin
Problem is a lot of WTFers out there would consider this as good code - why, it's got logging like a proper enterprise app!
Anyone else noticed the mysterious methodName variable at the top? Has to be a static member is it's a static method. Hope this isn't multi-threaded code.
Captcha: consequat - almost inconsequential?
Admin
Since it's a Java-program and Integer is part of the standard java library, a point where Integer.parseInt(myint.toString()) fails, it would be time to shutdown the application instead of just returning false. BTW: The point where these checks should take place is the unit-test and not the productive code.
Admin
This is the real WTF, if they want to get the NumberFormatExceptions, put that in the catch block, not Exception. They will be masking other errors with this catch.
Admin
sarge has TRWTF. If all the methods are using that same static variable then in all probability this system's logging is a complete waste.
Admin
They should have used annotations (in C#-ese, attributes) and reflection to avoid that ;).
Admin
Admin
Admin
This is definitely not CMMI level 5. I mean, what happens if the logger throws an exception? This code is missing a Logging Exception Manager Logger.
Admin
You're right.
What'll probably happen is that the logging function'll get messed up, assuming more than one thread can ever get CPU time with this program. All of that logging and it'll just get confusing due to the static member.
Admin
Ah, so the variable name should be called methodNameIsValidNumber, so we disambiguate it from all the other method names.
Well, but then there could be overloading, too, so it should be called methodNameIsValidNumberInteger, to make it more unique.
Oops, but then there could be a method named isValidNumberInteger(), so we can call it methodNameIsValidNumberParam1Integer, that would be totally unique.
Or we could just scope it locally, be done with it.
Admin
Trial by exception is bad enough - there are almost always ways to check for validity without relying on exception handling - but catching a top-level System.Exception when you're specifically looking for a FormatException is utterly retarded. The JVM could throw any number of internal exceptions (OutOfMemoryException for example) and this handler would pick them all up, even though it has no way of handling them and will actually misinterpret the results, thinking it's a FormatException when it could be something different.
This code offends me.
Admin
All you people don't understand. This is NOT a java.lang.Integer! They reimplemented it. Doh.
Admin
Furthermore, this really needs to catch Throwable!
Admin
Admin
Apparently I'll never be CMMI 5 because I don't like to make multiple log entries where nothing has happened. Reading this code I feel certain that in every one of the gazillions of places where this is called there is a call to log "Calling number format validator" and another when it returns, "Returned from number format validator."
After swallowing my revulsion and several antacids it occurred to me to question why they didn't log the value itself, or where this bloat had been called from. You know, stuff that might actually come in handy if you were trying to support this application in a production mode.
/CAPTCHA: augue - it's incredible, it's edible, it's what's for breakfast with bacon
Admin
There is no OutOfMemoryException, only an OutOfMemoryError, that is not catched by a catch(Exception)-block (for that you need to catch Throwable). The only other internal error that can occur in the line in the example is a NullPointerException which is the only possible value for the parameter that isn't a number.
Admin
I'm not all that familiar with java, but if a parameter is typed as integer, does it make sense to check inside the function if the parameter really is an integer? And while we're at that, the parameter is converted to a string, which is then evaluated for integer values, wait a second, this is not ported from someone's personal php project, is it.
Admin
Now they just need one other method:
Admin
There is no such thing as an OutOfMemoryException in Java. Please get your exception hierachy right before complaining. Not logging the exception is probably bad tho, you are right at that.
At first I didn't even see trwf in this one as I automatically assumed the parameter was String or maybe Object or something. Having it as Integer is really... great (hey, hotspot can probably optimize it ;)) Good catch on noticing that static field access, wouldn't have noticed that one either...
Admin
TRWTF is that the code in the try block throws NullPointerException when number is null, not NumberFormatException (well, actually, perhaps with an appropriate background thread that would change the locale of the jvm often enough...) Also, out-of-memory would not get caught like someone wrote, as OutOfMemoryError is, well, an Error and not an Exception
Admin
Admin
To know the answer you must be the answer.
Dive into your belly-button and seek out the Nirvana of Wisdom.
...
Belly-button. I cannot stress that enough. Do -not- dive any further south ... if you know what I mean.
Admin
CAPTCHA: jumentum - when a Rabbi is on a role.
Except, what balls and other round objects tend to do is ROLL.
Admin
Admin
I have a somewhat similar example to share.
I was once ordered to conduct a review of the C++ code sent in by our Subcompany from India (CMMI level 5 of course - what else?). The task of the class in question was parsing a configuration file in INI format, with strings as keys and integers as values. The class was implemented as a template class with 5 template parameters, e.g. one for line separator (newline character), one for the field separator (= symbol) etc. But the most evil thing was the method parseLine() of that class. This method accepted the line as a reference to a C++ string, used the c_str() method on that one, subsequently casted the const away, and passed the resulting char* to good old strtok().
Somehow they did not understand my comment "this code is dangerous and must be rewritten"....
Admin
This is nothing - At a company I worked for several years ago, we brought in a contractor to write some code for us. He had constructors with 2-3 THOUSAND LINES of code in them!
Then we'd look for various methods on his objects, they were almost non-existent, and when they did exist, they were usually of the "GetByID" kind of methods which did very little and would normally be found....in a constructor!
We had a lot of fun cleaning up that code
Admin
... is that the logger always produces "Returned true" messages.
Admin
Admin
Admin
When does knowing the fundamentals of a programming language and it's environment make someone a fanboy rather than competent? At least you proved you're a fuckwit though.
As for the code in the article, I love the way the method name is logged via a local string - most logging libraries default to using reflection to get the method name automatically.
Admin
My TRWTF list:
Method is useless, and will probably fail only for null. And it will log misleading information in that case.
methodName seems to be a property. Scary, as already pointed out and explained.
Method catches Exception. Wow. So it will consider a format problem any exception that might happen. The comment and the log even mentions "NFE", so what the hell?
Although it is logging all kinds of crap, it won't log the stacktrace. It would be perfectly fine not to log it if they were catching only format problems. Actually, that's the correct way, not to flood log with expected (expected as in handled) stacktraces. But whenever you catch a generic exception, not logging it is a very serious mistake.
The information it's logging is far from ideal, even when forgetting about the stacktrace. It has found out that a number is not valid, but won't print the number, only a "returned false". Unless it's very sensitive information, it should to be logged (no invalid passwords in log, for instance).
Printing "entering" and "exiting" for such a trivial method? There's no reason for that. If you're ever caught in a situation where it's useful, you should be profiling instead of parsing gigabytes of logs.
Printing the class and method name that way? Really? Log4j will add class, method, and even thread names on your log messages, without any additional logging code on your application. Imagine some programmers copying and pasting these logs to other methods and forgetting to change the variable names? Log will be fun.
It's so bad I can't even find it funny. Bad error handling leads to a lot of headache and frustration.
Admin
We had the same experience with our CMMI Level 5 offshore-team. These guys had great processes that forced them to write
instead of
But somehow, the automatic process did not cover the fact, that you should not name html-session-keys "Flag1", "Flag2" and "Flag3" :(
Admin
When will management people understand the only thing to outsource is their a$$e$... -_-
Admin
I ran into a version of this when inheriting a large Delphi application:
What value can an int have (in Pascal: no null on primitives) that did not involve digits?
It was the headliner in my Menagerie of Regrettable Code. Sadly, I had over 20 exhibits.