• (cs) in reply to Charles Nadolski
    Charles Nadolski:

    Yes, the proper way would be to check the string beforehand.

    Let's assume you need a floating point number instead of an integer, just to make it more funny.
    Then you will have a lot of fun to make sure your checking function knows every special case,
    like "-1234.45e-2". But then again, there is still a chance that Double.parseDouble() will not accept some number you try to parse, or your function does not accept every input Double.parseDouble() accepts. So, in the end, just because you don't like the NumberFormatException, you duplicate work done by the programmers of the JVM. But guess what: Java forces you to eighter catch or rethrow the NumberFormatException anyway!

    So your program looks like this:

    String tempString = request.getParameter("temperature");
    double temp;

    if (!StringIsDouble(tempString)) {
      // handle error
    }
    else {
      try{
          temp = Double.parseDouble(tempString);
      }catch(NumberFormatException e){
          // NO NO THIS CAN'T HAPPEN! I checked it!!!
      }

    }

    Charles Nadolski:

    The easy way would be to ask yourself the question "Will the accountParam always be greater than zero"?  At least in C++, atoi(strSomeCString) will return zero if the string is not a number.  Check if accountID is zero.  This should handle a large number of cases.

    Wrong. atoi("1234abc") returns 1234 but "1234abc" is not a valid number.
    Even worse, atof("1234,567") returns 1234.0 because someone mistakenly used , as decimal sperator, which is not unlikely e.g. in Austria or Germany.
    Charles Nadolski:

      Otherwise, write a function that checks if it's a number, like bool StringIsNumber().  Not only is it a good learning experience, a function like that can  be used in a lot of places.  Also this function can be written to be very fast too, much much faster than parseInt I imagine.

    Oh yes, let's reinvent the wheel. If 9 out of 10 strings to parse are not valid numbers, the overall speed might be a bit better (depending on how parseInt is implemented). Otherwise, the strings get parsed twice, so it will be much slower.
  • Candle (unregistered)

    Exceptions are very useful when you are loading other files from java:
    a lot of the java.io.* class methods throw an IOException, now, do you want to write:

    FileReader fw;
    try {
        fr = new FileReader(new File("foo.txt"));
    } catch (FileNotFoundException) {
        fr = null;
    }
    

    or

    File f = new File("foo.txt"));
    FileReader fw;
    if (f.exists()) {
        fw = new FileReader(f);
    }
    

    I would chouse the first, to me it seems much cleaner and neater, particularly if you expect the file to exist (it is part of the package), however, both have their uses, in some occasions, you want the "if" (the file is user-supplied) in others you want the try/catch (your packaged file)

  • (cs) in reply to ammoQ

    AmmoQ precisely!

    It just seems perverse to re-write all this code to avoid using Exceptions!

    And personally, from a maintainability point of view, I'd much prefer to receive a nice Exception object, with custom fields providing more info, and a nice descriptive name than an error code.

    Which is friendlier? SQLException with a nice message and stacktrace, or '3' which I have to muck about looking up myself - that is after checking that it's an error code I'm getting back, not the actual return I wanted!.. no contest. As a rule, so what if the program spends a little more time, better than me having to look up what some number actually means.

    Using a custom Exception class means that not only can I add more info to help code further up the stack decide what to do, but I can also be reasonably sure that some future programmer isn't going to code in the same error number by mistake and get things horribly confused.

    I don't know what kind of code Joel in the link above has to write - but I know that there are many, many cases where an problem cannot be dealt with at the source - sometimes that object just needs to delegate it up.


  • (cs) in reply to Charles Nadolski

    Charles Nadolski:

    Yes, the proper way would be to check the string beforehand.

    Parse it before you parse it.

    Charles Nadolski:

      The easy way would be to ask yourself the question "Will the accountParam always be greater than zero"?  At least in C++, atoi(strSomeCString) will return zero if the string is not a number.  Check if accountID is zero.  This should handle a large number of cases.

    What does it return when the input is "0"?

  • (cs) in reply to Candle
    Anonymous:
    File f = new File("foo.txt"));
    FileReader fw;
    if (f.exists()) {
    fw = new FileReader(f);
    }

    This version is vulnerable to race conditions, since the file could "vanish" between f.exists() and FileReader(f), causing a probably uncaught exception. Another plus for the version with the exception.
  • (cs) in reply to ammoQ

    And of course f.exists() doesn't imply that FileReader(f) will be successfull; missing access rights could prevent it. For all that reasons, people who don't use exception and try to do every possible test by themself will write programs that are larger, slower, harder to read and more likely to fail. WTF

  • (cs) in reply to ammoQ

    ammoQ:
    And of course f.exists() doesn't imply that FileReader(f) will be successfull; missing access rights could prevent it. For all that reasons, people who don't use exception and try to do every possible test by themself will write programs that are larger, slower, harder to read and more likely to fail. WTF

    I see catch blocks like a net that I can use to for any problems that shake out of the code.  They are great because yyou can deal with things once or at least in a reduced number places and most importantly where it makes the most sense.  And you don't need to create a catalog of error codes and what they mean.  I literally have to pull out a book and look up what E8 means for a given transaction with some old code right now.  It's so stupid I can't even think about it without my skin crawling.

    The greatest thing about exceptions for developers is that you can see the stack trace.  I've save countless hours of debugging time by just using the stack trace.

    And it just gets better.  You can nest your exceptions so that not only do you know what went wrong, you often know why.

  • G McDorman (unregistered) in reply to evnafets
    evnafets:

    Anonymous:
    loneprogrammer:
    All that needs to change is

    "[a-z]" --> "[a-z]*"
    Actually, no. '*' means zero or more, so "[a-z]*" will match anything, even a string of non-letters.

    What will work is:
     "^[a-z][a-z]*$"
    or
     "^[a-z]+$"

    Yet again there seem to be as many WTF comments as there are in the original post.  loneprogrammer was correct.  I don't know if G McDorman and Jonathan Pryor are just trolling, but they seem serious enough

    You are correct; I apologize for the error. I wasn't attempting to troll; I was speaking in terms of what I know, which is general regular expressions. I haven't used Java pattern matching, so I missed this in the documentation of the Match class:

    • The matches method attempts to match the entire input sequence against the pattern.

    You're also correct in that the original method would match an empty string; I overlooked that the first time.

    It's still probably an inefficent way to solve the problem, though; a loop testing for isLetter/isLowerCase would probably be better (both in speed and memory consumption).

    With respect to the '+' regular expression modifier, I was thinking of non-Java regular expression implementations, such as in Unix implementations of sed, vi, and others. Most GNU (i.e. Linux) versions of these likely have the '+' modifier; Unix (e.g. Solaris) often don't.

  • diaphanein (unregistered) in reply to dubwai

    The thing people often overlook with exceptions is that you should always catch the most specific exception possible, and only those that you can handle.  Doing a catch-all will almost always lead to a condition of confusion and hours, if not days of debugging and tracing.

    Best to take a defensive approach, especially when writing libraries for use by others.  ALWAYS validate input parameters.  If they fail the tests throw.  Don't try and recover (unless you can do so intelligently).  When throwing, always throw the most detailed message you can, and if possible the data that led to the throw.  Nothing worse than getting a default constructed ArgumentException thrown from 20 levels deep in a stack trace in code that isn't yours.  (This happened to me while using MS's managed DirectSound library).

    There are, however times when throwing and catching is actually faster than checking ahead of time for a possible failure.  In python, a well accepted idiom is to do something like the following:

    try:
       value = dict[key]
    except KeyError:
       dict[key] = new_value

    Instead of doing:

    if dict.has_key(key):
       value = dict[key]
    else:
       dict[key] = new_value

    One reason for this is that it is typically faster to to fail on the lookup into the dictionary and catch exception, than it is to check for existence first (considering checking for existence, then retrieval requires 2 lookups into the dictionary instead of 1).  This is especially true if the general case is you expect the value to exist.

    Exceptions have their place.  Message boxes have their place.  Taken to either extreme, problems result.  Just like programming construct.  The key is intelligent use of both constructs.

  • onyxjl (unregistered) in reply to diaphanein

    I like to follow these principles...

    http://www.codeproject.com/dotnet/exceptionbestpractices.asp

     

  • nonDev (unregistered)

    I always liked the doctor analogy. If a patient walks in complainig of intermittent muscle stiffness, your first reaction should not be to think of nondystrophic myotonia and start running tests. Neither your patients nor their insurance companies will be grateful.

  • (cs) in reply to onyxjl
    Anonymous:

    These are good.  The only 'exception' I take with this is the advice that you should never put in an empty catch block.  It's actually good advice for newbies and technically I agree, you should put in a comment if you don't plan to do anything but there are cases where you really don't care about the exception and the most appropriate action is to do nothing.  Normally, I would use a Logger and write the exception as a debug statement but there are cases where that isn't appropriate either.

    The best example I have is that the Oracle JDBC driver throws an exception when closing a connection that is already closed.  This is obnoxious and I have seen it actually cause bugs.  So when cleaning up the DB resources, it's best to put each command in a separate try catch block to make sure all commands are attempted and all Objects released.

    OK, so maybe it should still log the error but in practice this just leads to a bunch of log statements no one cares about.  If you use a logging framework, you can just turn those off so in that case I will write them out as debugging.

  • onyxjl (unregistered) in reply to dubwai

    There are certainly some cases where one can make an argument to just eat an exception (although you should put a comment). There are also situations where you might deviate from some of the other guidelines as well.

    Like all best practices though, these exception best practices form a good starting guide that you really should have a solid reason for deviating from, and this should also be relatively uncommon. It seems as though you are in agreement with that.

    It is certainly better to say "Never eat an exception" and let people argue for the special case than it is to say "Try not to eat exceptions" and open the door for people to start looking for excuses to be different. IMO, if you open the door to lazy short-cutting then people will walk through it.



  • (cs) in reply to dubwai

    Java sometimes forces you to catch an error that can not possibly ever be thrown.
    Look at the original WTF: The PatternSyntaxException will not happen for that constant string. Never ever.  Another example are Interfaces which declare that they might throw an exception; but you actual implementation of the interface doesn't.
    So what's the point in putting anything at all into that catch block?  The only plausible action I can think of is to throw a RuntimeException to help finding programming errors.

  • (cs) in reply to ammoQ

    ammoQ:
    Java sometimes forces you to catch an error that can not possibly ever be thrown.
    Look at the original WTF: The PatternSyntaxException will not happen for that constant string. Never ever.  Another example are Interfaces which declare that they might throw an exception; but you actual implementation of the interface doesn't.
    So what's the point in putting anything at all into that catch block?  The only plausible action I can think of is to throw a RuntimeException to help finding programming errors.

    Wrapping it in a Runtime isn't a bad choice.

    LOGGER.error("should not happen", e);

    is another.

  • AssertiveOne (unregistered) in reply to dubwai

    Translating it into some RuntimeException is one good solution (maybe an IllegalStateException?), logging it somewhere is another, and

    assert false : "unable to compile pattern for regular expression";

    is the baby that makes three.

  • Ken (unregistered)

    Wow. the link to MSDN no longer works... the real WTF of this story is that. Only the Japanese version of the page for "On Error Resume Next" is available. WTF?!

Leave a comment on “Try. Catch. Throw Up.”

Log In or post as a guest

Replying to comment #:

« Return to Article