• TK (unregistered)

    Frist...

  • Quite (unregistered)

    Looks like someone had a difficult job of getting it to work where it crashed in certain circumstances, and clearly couldn't be bothered to tidy up after him/herself after putting all the appropriate logic into doCalculation.

  • Balr0g (unregistered)

    But why should the request data be modified?

  • Frank (unregistered) in reply to Balr0g

    It shouldn't be modified, but if I didn't explicitly say so, someone would have pointed out that we didn't know for sure that request.includeFooFlag() might not have changed and maybe the lack of else if was deliberate. Which is not the case.

  • YourNameHereJustNineNinetynine (unregistered)

    But that only handles the true- and false-cases.

    What if request.includeFooFlag() is FileNotFound?!

  • (nodebb) in reply to YourNameHereJustNineNinetynine

    FileNotFoundException, duh!

  • Andreas (google) in reply to dkf

    Actually, "Error Details"

  • Brian Boorman (google)

    So, basically:

    Response response = new Response();
try {
    response = doCalculation(request);
} catch (Exception ex) {
    response = createErrorResponse("Error details");
}
return response;
  • Melnorme (unregistered) in reply to Brian Boorman

    You can do better than that.

  • Charles Wood (google) 
    try {
    return doCalculation(request);
} catch (Exception ignored) {
    return createErrorResponse("Error details");
}

    Addendum 2016-09-28 09:46: Doh, doCalculation has its own exception handler... /facepalm

  • Sham (unregistered)

    Another example of why Java is horrible.

  • Charles Wood (google)

    This definitely reads like code that either used to do different things, or where the developer just couldn't figure out what was going on and so kept adding tests until the error went away (probably due to an unrelated edit). Good times.

  • Onan (unregistered) in reply to Brian Boorman

    ..or

    try {
   return doCalculation(request);
} catch (Exception ex) {
    return createErrorResponse("Error details");
}
  • Sandman (unregistered) in reply to Brian Boorman

    Well, according to the article, this would suffice: return doCalculation(request);

  • Sandman (unregistered) in reply to Sham

    I don't think you can blame the language here.

  • isthisunique (unregistered) in reply to Charles Wood

    Or where the developer intended to have something more generic but in the end never pulled it up a layer. The outer exception case might make sense. I replace all unhandled unsafe exceptions with a wrapper exception that is safe to pass up through IO. This will be wrapped around any function called which happens in a style akin to RPC. It's natural in that case that you're going to have it even wrapping methods that don't through an exception or don't need to try catch.

    I still think a failure to clean up is more likely. doCalculation having its own try catch doesn't sound good.

  • Brian Boorman (google) in reply to Charles Wood

    We won't get into the debate about where function return statements should go.

  • satan (unregistered)

    This is pretty common. You write a block of code that has two branches. Then through refactoring and general code changes, the if/else blocks do the same thing. But it never gets cleaned up. I'm sure the code several revisions ago probably looked a lot more logical. Granting catching Exception is usually a very bad thing unless you have a very good reason for it. And in that case, you should catch the specific runtime exception. Now things can go wrong but you don't know why especially without logging.

  • The "Mr" (unregistered) in reply to Onan

    Unless request.includeFooFlag() will modify the result of doCalculation(request), or throw an exception, or change values, three possibilities that should not go overlooked.

  • (nodebb)

    Obviously the project lead told the programmer to write error details into the response in case of an exception.

    Anyway, these are only 24 lines of code. I've seen similar contraptions stretching over hundreds of lines.

    If rollout is due the day after tomorrow, and you know there's still a bug in a (really big) code block, and you are expected to avoid an unhandled exception at all cost, what would you do? (except for a comment why you introduced the try block, and an error message with more than half a bit of information)

    On a very loosely related note, I'm glad I didn't encounter the exception handling equivalent of if ... else if, that is, try ... catch try. So far.

  • Norman Diamond (unregistered)

    Then through refactoring and general code changes, the if/else blocks do the same thing. But it never gets cleaned up.

    But should it be cleaned up?

    After a few more general code changes, the if/else blocks won't do the same thing any more. Will that be due to intentional changes, or coffee & pasta failures?

  • (nodebb)

    Today's "Internet Rant on Today's Featured Language" post. http://tech.jonathangardner.net/wiki/Why_Java_Sucks

  • (nodebb) in reply to Watson

    I've just read that incompetent rant and I want my 5 minutes back.

  • Frank (unregistered) in reply to PWolff

    "Anyway, these are only 24 lines of code. I've seen similar contraptions stretching over hundreds of lines."

    This is simplified and cleaned up a lot because it took me the best part of an hour to make sense of the original and figure out that it was as redundant as it looked.

    For instance, before every return point there's about 15 lines including another DAO call and ANOTHER return point. And I omitted three more layers of if/else with similar code between the outer try{} and the portion of the innards shown here. The whole thing is just shy of 200 lines, and the two if() blocks make up over half of that.

  • Frank (unregistered) in reply to Frank

    Also, it doesn't literally say "error details". But it is a hard-coded and only slightly more informative string that's the same everywhere.

  • Charles Wood (google) in reply to isthisunique

    "I still think a failure to clean up is more likely. doCalculation having its own try catch doesn't sound good."

    Yeah, I like to have the exception handling placed as high up the stack as possible. But since we don't know anything about it, we can't really tell if it's good. What if doCalculation has a side effect (e.g., I/O or network comms), and it needs to be able to handle layer-specific exceptions without passing them up to its callers?

    (Why a function named doCalculation would have side effects is another issue.)

    (Also whoever wrote this code doesn't seem likely to be that concerned with proper design.)

  • ATF (unregistered) in reply to Charles Wood

    This is debatable. We don't know the public/private nature of the other method. Unless doCalculation is a private method, the callee should NOT assume it is handling the try/catch logic. This looks to be more of a re-factor that was never cleaned up as the other person suggested.

    There's nothing worse than one method RELYING on the fact that another method is 'taking care' of the exception handling, only to have another person re-factor that method because it's being called by yet a third method and they wanted access to the Exceptions being thrown. Bam, now doCalculation throws and crashes the web serivce. Where as at least the original WTF code will still work.

    Sometimes the code in WTF is not a WTF and just someone wanting to be a narcissistic jerk and join in on a good ol' fashion circle

  • Anonymouse (unregistered) in reply to Sham

    Another comment why you are stupid

Leave a comment on “Every Possible Case”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article