The try{ throw(...); } catch(...){ log(...); } pattern



  •  Just found this gem in some code:

     

     

    try {
        if (!backupDir.mkdir()) {
            throw new Exception("");
        }
    } catch (Exception e) {
        logger.error("Could not create backup directory "
                   + backupDir.getAbsolutePath());
    }
    


  • Why just use a simple if statement when you can waste extra time creating a new variable in scope, as well as throwing and catching an error? This way you can make it a speedup loop later.



  • If File.mkdir() fails, it can either throw a SecurityException or just return false. TRWTF is Java for encouraging this anti-pattern.



  • (sorry for the C# bool :p)

    So something like this:

    bool success;
    
    try
    {
      succes = File.mkdir("bla");
    }
    catch (SecurityException)
    {
      success = false;
    }
    
    if (!succes)
      logger.log("could not create bla");
    


  • Shall we excuse the succes/success typo or not? Raise your hands!



  • @Zecc said:

    Shall we excuse the succes/success typo or not? Raise your hands!

    NEVER! That's why we have the Preview tab, oh...



  • This code is not as stupid as it seems. Because mkdir can signal "directory creation failed" by either throwing an exception or by simply returning false. And if you want to handle theese 2 cases without duplicating the error.logger(Error handling) call then this is the way to do it.

     The true wtf is that if the directory exists, mkdir will return false and log an error saying "Could not create backup directory", but if the there is a file_exists() someware before then this might be the best way to do it. (And yes the true wtf is that java can signal error either by exception or by returning false(And how do you find out why it could not create the directory if it just return false??)

     

     



  • @tiller said:

    This code is not as stupid as it seems. Because mkdir can signal "directory creation failed" by either throwing an exception or by simply returning false. And if you want to handle theese 2 cases without duplicating the error.logger(Error handling) call then this is the way to do it.

     The true wtf is that if the directory exists, mkdir will return false and log an error saying "Could not create backup directory", but if the there is a file_exists() someware before then this might be the best way to do it. (And yes the true wtf is that java can signal error either by exception or by returning false(And how do you find out why it could not create the directory if it just return false??)

    No, TRWTF is that it doesn't matter whether the mkdir succeeds or not, succes is (almost) always set to true, both here and through the rest of the code.  There's only one line which could set it to false, and that's in a block That Could Never Be Reached, so it'll never log.  Except, of course, that when the block is reached, it's in another thread that then gets stuck there, so then it always logs, even when the mkdir does succeed.

    Good job that it only logs the error, but doesn't otherwise change the flow control, so that backups continue to work, even when it is claiming they're not (or should be claiming they're not.)

    (You mean there's a *reason* why mkdir sometimes fails?  It's not just random?)



  • @tgape said:

    No, TRWTF is that it doesn't matter whether the mkdir succeeds or not, succes is (almost) always set to true, both here and through the rest of the code. [...]


    WTF are you on about? There are two cases where succes is set to false: mkdir() throws an exception or returns false (presuming tiler is correct). Also, I'll presume your last line is orange text so I won't mention the nine possible ways mkdir can fail.



  • @tgape said:

     

    (You mean there's a *reason* why mkdir sometimes fails?  It's not just random?)

     

    • out of inodes
    • permissions
    • read-only medium
    • no parent directory
    • server down/disk hit with hammer


  • @davidrhoskin said:

    @tgape said:

     

    (You mean there's a *reason* why mkdir sometimes fails?  It's not just random?)

     

    • out of inodes
    • permissions
    • read-only medium
    • no parent directory
    • server down/disk hit with hammer

    Sigh.  Apparently, I'm not sufficiently skilled at inserting insanity into my posts to be able to skip tags with impunity.  Also, you missed cosmic rays, drive glitches, and "we secretly replaced your normal operating system with Folger's Crystals".

    (For what it's worth, error messages that don't bother to pass on the actual error are one of my personal pet peeves.  It's also the most common reason why I veto code, so another pet peeve is "cow orkers who can't seem to learn how to write code that will pass review the first time.")

    @Lingerance said:

    Also, I'll presume your last line is orange text so I won't mention the nine possible ways mkdir can fail.

    I'm not sure what you mean by orange text, but it may be entirely possible the entire response was orange text.



  • @tgape said:

    I'm not sure what you mean by orange text

    Orange text == sarcasm


  • Discourse touched me in a no-no place

    @tgape said:

    @Lingerance said:
    Also, I'll presume your last line is orange text so I won't mention the nine possible ways mkdir can fail.

    I'm not sure what you mean by orange text,

    I presume it's a reference to this


  • @Zecc said:

    Shall we excuse the succes/success typo or not? Raise your hands!
    Well, he seems to be from Belgium, so presumably his mother tongue is Dutch, where success is written succes, so I'm inclined suggest to let this one slip. (hey you asked!)



  • @Lingerance said:

    @tgape said:
    I'm not sure what you mean by orange text

    Orange text == sarcasm
    WTF?



  • @tster said:

    @Lingerance said:
    Orange text == sarcasm
    WTF?
     

    It's a thing people do.

    It's juuust not well-known enough for a portion to not go WTF like you did.



  • @RogerWilco said:

    @Zecc said:

    Shall we excuse the succes/success typo or not? Raise your hands!
    Well, he seems to be from Belgium, so presumably his mother tongue is Dutch [ . . . ]

    Ah, so "Orange" text == Dutch ?




  • @DaveK said:

    Ah, so "Orange" text == Dutch ?
     

    Yes. Now move along please.



  • @dhromed said:

    @DaveK said:
    Ah, so "Orange" text == Dutch ?

    Yes. Now move along please.

    Sweet, I can read dutch now!



  • @belgariontheking said:

    @dhromed said:

    @DaveK said:
    Ah, so "Orange" text == Dutch ?

    Yes. Now move along please.

    Sweet, I can read dutch now!
    What did dhromed say about my mother?


  • @bstorer said:

    What did dhromed say about my mother?
     

    Your mother is a classy lady.

    Total scrag.



  • @dhromed said:

    @bstorer said:

    What did dhromed say about my mother?
     

    Your mother is a classy lady.

    Total scrag.

    Belgarion can read that, you know.

     



  • @DaveK said:

    Belgarion can read that, you know.
     

    I know. I was referring to Bstorer's mom.

    Belgarion's mom is totally sweet. You know. In the sack.

    pstorer is an idiot



  • @dhromed said:

    @DaveK said:

    Belgarion can read that, you know.
     

    I know. I was referring to Bstorer's mom.

    Belgarion's mom is totally sweet. You know. In the sack.

    Belgarion can read that too, you know.

    @dhromed said:

    pstorer is an idiot

    U R DOIN IT WORNG.  You're meant to use secret writing for things that everyone doesn't already know, goddammit!

Log in to reply