• (cs)

    You gotta love "Log Driven Programming".

  • Thomas (unregistered)

    If an Integer is not parsed is it still an Integer?

  • (cs)

    When exactly will this function return False?

  • anonymous (unregistered)

    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.

  • SR (unregistered)

    I'll bear this method in mind if I ever get paid by line of code.

  • highphilosopher (unregistered)

    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!

  • ath (unregistered)

    I wonder if you could write a "log-file-compiler" taking the log file as input and producing a fully functional program?

  • Thomas (unregistered) in reply to steenbergh
    steenbergh:
    When exactly will this function return False?

    When the Integer is NULL. I don't think NULL is a valid number. :)

  • (cs) in reply to Thomas
    Thomas:
    steenbergh:
    When exactly will this function return False?

    When the Integer is NULL. I don't think NULL is a valid number. :)

    Yup, you're right. This, however, wouldn't be my method of choice to test for NULLs.

  • (cs) in reply to Thomas
    Thomas:
    steenbergh:
    When exactly will this function return False?

    When the Integer is NULL. I don't think NULL is a valid number. :)

    That's one hell of an expensive null-checker.

  • ToAshamedToName (unregistered)

    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?

  • Steve the Cynic (unregistered) in reply to frits
    frits:
    Thomas:
    steenbergh:
    When exactly will this function return False?

    When the Integer is NULL. I don't think NULL is a valid number. :)

    That's one hell of an expensive null-checker.

    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.

  • Lothar (unregistered) in reply to steenbergh

    When the passed parameter is null

  • (cs) in reply to steenbergh
    steenbergh:
    When exactly will this function return False?
    When the ToString() method produces something the parseInt() method cannot parse, or throws an exception all by itself (say, out of memory). So it's a really good check of whether the library programmers have done a good job. It will trigger some unwanted behavior at a random moment if the library programmers overlooked a small thing. Awesome.
  • Azeem (unregistered)

    That's what happens when you pay per line for outsourcing!

  • Lothar (unregistered) in reply to Lothar
    Lothar:
    When the passed parameter is null
    This was meant as answer to the question "when the method will return false". Forgot to added the quoting block.
  • sarge (unregistered)

    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?

  • Lothar (unregistered) in reply to TGV
    TGV:
    steenbergh:
    When exactly will this function return False?
    When the ToString() method produces something the parseInt() method cannot parse, or throws an exception all by itself (say, out of memory). So it's a really good check of whether the library programmers have done a good job. It will trigger some unwanted behavior at a random moment if the library programmers overlooked a small thing. Awesome.

    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.

  • Fenris (unregistered) in reply to Steve the Cynic
    Steve the Cynic:
    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.

    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.

  • NoAstronomer (unregistered)

    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.

  • (cs) in reply to sarge
    sarge:
    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.

    They should have used annotations (in C#-ese, attributes) and reflection to avoid that ;).

  • (cs) in reply to steenbergh
    steenbergh:
    Thomas:
    steenbergh:
    When exactly will this function return False?

    When the Integer is NULL. I don't think NULL is a valid number. :)

    Yup, you're right. This, however, wouldn't be my method of choice to test for NULLs.

    Agreed; a proper test for null requires xml!

  • (cs) in reply to sarge
    sarge:
    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?

    Erm, why? Do your methods change names while running? (assuming it's not changed for each method)
  • Nibh (unregistered) in reply to frits

    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.

  • ClutchDude (unregistered) in reply to sarge
    sarge:
    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?

    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.

  • programmer (unregistered) in reply to sarge
    sarge:
    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.

    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.

  • Anonymous (unregistered)

    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.

  • prionic6 (unregistered)

    All you people don't understand. This is NOT a java.lang.Integer! They reimplemented it. Doh.

  • prionic6 (unregistered)

    Furthermore, this really needs to catch Throwable!

  • Quirkafleeg (unregistered) in reply to programmer
    programmer:
    Ah, so the variable name should be called methodNameIsValidNumber, so we disambiguate it from all the other method names.
    So… it's for checking whether method names are valid numbers?
  • 3rd Ferguson (unregistered) in reply to ath
    ath:
    I wonder if you could write a "log-file-compiler" taking the log file as input and producing a fully functional program?

    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

  • Lothar (unregistered) in reply to Anonymous
    Anonymous:
    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.

    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.

  • (cs)

    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.

  • (cs)

    Now they just need one other method:

    public static boolean isValidCode( String sourceCode )
    {
        Logger logger = LoggerHelper.getLogger( LOGGER );
    
        logger.info( "This code sucks." );
    
        return false;
    }
  • Frz (unregistered) in reply to Anonymous
    Anonymous:
    The JVM could throw any number of internal exceptions (OutOfMemoryException for example) ...

    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...

  • Anonymous Coward (unregistered) in reply to Fenris
    Fenris:
    Steve the Cynic:
    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.

    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.

    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

  • Anonymous (unregistered) in reply to Frz
    Frz:
    Anonymous:
    The JVM could throw any number of internal exceptions (OutOfMemoryException for example) ...

    There is no such thing as an OutOfMemoryException in Java. Please get your exception hierachy right before complaining.

    Boo fucking hoo, my point is exactly the same. The JVM could throw any number of system exceptions and the exception handler in today's article will pick them up despite having no means to handle them. Get over yourself you silly little Java fanboy.

  • Edward Royce (unregistered) in reply to Thomas
    Thomas:
    If an Integer is not parsed is it still an Integer?

    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.

  • Bgglw (unregistered) in reply to anonymous

    CAPTCHA: jumentum - when a Rabbi is on a role.

    Except, what balls and other round objects tend to do is ROLL.

  • John (unregistered) in reply to steenbergh
    steenbergh:
    This, however, wouldn't be my method of choice to test for NULLs.
    +1 Funny
  • rink (unregistered)

    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"....

  • Kelly_Phillips (unregistered)

    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

  • doesnotmatter (unregistered)

    ... is that the logger always produces "Returned true" messages.

  • Anonymously Yours (unregistered) in reply to ath
    ath:
    I wonder if you could write a "log-file-compiler" taking the log file as input and producing a fully functional program?
    Hm. Perhaps you could combine that with the Whitespace programming language to achieve CMMI6.
  • Ouch! (unregistered) in reply to Anonymous
    Anonymous:
    Frz:
    Anonymous:
    The JVM could throw any number of internal exceptions (OutOfMemoryException for example) ...

    There is no such thing as an OutOfMemoryException in Java. Please get your exception hierachy right before complaining.

    Boo fucking hoo, my point is exactly the same. The JVM could throw any number of system exceptions and the exception handler in today's article will pick them up despite having no means to handle them. Get over yourself you silly little Java fanboy.
    Your point stands only partially, the catch block catches Exceptions, what the JVM throws when something goes really wrong are Errors. So it's not quite as catastrophic as it could be.

  • (cs) in reply to Anonymous
    Anonymous:
    Frz:
    Anonymous:
    The JVM could throw any number of internal exceptions (OutOfMemoryException for example) ...

    There is no such thing as an OutOfMemoryException in Java. Please get your exception hierachy right before complaining.

    Boo fucking hoo, my point is exactly the same. The JVM could throw any number of system exceptions and the exception handler in today's article will pick them up despite having no means to handle them. Get over yourself you silly little Java fanboy.

    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.

  • Ale (unregistered)

    My TRWTF list:

    1. Method is useless, and will probably fail only for null. And it will log misleading information in that case.

    2. methodName seems to be a property. Scary, as already pointed out and explained.

    3. 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?

    4. 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.

    5. 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).

    6. 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.

    7. 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.

  • Andreas (unregistered)

    We had the same experience with our CMMI Level 5 offshore-team. These guys had great processes that forced them to write

    if (a == b){
      return "something";
    }
    return "soemthingElse";
    

    instead of

    if (a == b){
      return "something";
    } else {
      return "soemthingElse";
    }
    

    But somehow, the automatic process did not cover the fact, that you should not name html-session-keys "Flag1", "Flag2" and "Flag3" :(

  • (cs)

    When will management people understand the only thing to outsource is their a$$e$... -_-

  • Richard C Haven (unregistered)

    I ran into a version of this when inheriting a large Delphi application:

    if intToStr(anInt) = "" then

    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.

Leave a comment on “isValidNumber()”

Log In or post as a guest

Replying to comment #:

« Return to Article