• (cs)

    This is a good example of why you really need to be able to fire people.  It's so sad how infrequently idiots actually do get fired.

  • Will I Am (unregistered) in reply to JohnO

    If I ever get that way, some one please fire me.

  • Martin Carolan (unregistered)

    Knowing why the argument was invalid is for wimps, all we need to know is that it is and that's the end of that.

  • Aim Here (unregistered)

    In other news, the QA team is pleased to report that the number of instances of Invalid Arguments reported by this program has decreased by over 91% since Jed took a look at the code. Way to go, Jed!

  • Joe (unregistered) in reply to Martin Carolan

    My favourite is that it's only the case when invalidArgs == 12. So only if 12 bad things have happened do we tell them something is wrong. :)

  • boa13 (unregistered)

    Not only does he remove the detailed error messages, it seems he also will only throw an exception in the unlikely case that all tests are triggered at the same time. Too bad we can't see the 8 other tests he botched.

    Senior programmer? I'd rather say senile programmer.

  • (cs) in reply to Joe

    It's worse than it looks. These tests can never all succeed. The expansionRate can never be:

    1. Equal to 1.
    2. Less than 1 or greater than 6.

    Thus, at least two of the possible invalid arguments are mutually exclusive, and this will never report an error. The entire snippet can be replaced with:

    /* Validate arguments */

  • (cs) in reply to JohnO

    No! don't fire him! Keep him writing software applications because he used to be writing operating systems and if you fire him, he'll wind up writing operating systems again.

    Didn't you know that on the first versions of DOS, when you wrote a batch file with a typo, the error message came up as "unable to find file or command: xcpoy" which would help you locate your typo? After Jed was done with it, you got "bad command or filename". He then went on to work on the dot net framework so that constraints failed on a dataset, you get "one or more constraints failed" (thankfully, there is a workaround to get the useful messages out of the dataset when you get a bad result set) instead of "Column uid_employee on table employee_roles must not be null". Keep him writing code instead of standardized frameworks or operating systems. [+o(]

    I never thought I would say this, but thank goodness Jed didn't run the unit tests! otherwise he might have gone and changed the tests to pass and it would be harder to find the problem. Reminds me of the guy I worked with who would comment out a test as soon as it failed...

    BTW, I really really like the original style of error message which tells the user how to fix the problem rather than saying what the problem is. so much better to say "coefficient must be between 1 and 6 inclusive" rather than "coefficient cannot be less than 1 or greater than 6".

  • (cs)

    <!--StartFragment --> 

    Alex Papapapasomething:

    ...all 250+ opening curly braces being moved to the same line as the method/if/while statement...


    That's enough to piss me off.  I find that very unreadable.

    I'm willing to bet that Jed is a C-snob that thinks exceptions are stupid and that error codes are the only way to go.

    [image]

  • (cs) in reply to seebs
    seebs:
    It's worse than it looks. These tests can never all succeed. The expansionRate can never be: 1. Equal to 1. 2. Less than 1 or greater than 6.

    Thus, at least two of the possible invalid arguments are mutually exclusive, and this will never report an error. The entire snippet can be replaced with:

    /* Validate arguments */



    You can't say that becuase you don't know how many tests there are...

  • David P. Murphy (unregistered) 

    														If I ever get that way, some one please set me on fire.<br>

    ok
    dpm
  • (cs) in reply to Ytram
    Ytram:

    <!--StartFragment --> 

    Alex Papapapasomething:

    ...all 250+ opening curly braces being moved to the same line as the method/if/while statement...


    That's enough to piss me off.  I find that very unreadable.

    I'm willing to bet that Jed is a C-snob that thinks exceptions are stupid and that error codes are the only way to go.

    [image]

    Quit mocking the underprivileged pizza-less readers, you insensitive bastard.

  • vhawk (unregistered)

    Improved by what ? This must have been the village idiot - either 12 errors or none - now that is clever - and why would one want meaningful exception messages - those are for wimps.  Freaks me out looking at this !

  • (cs)

    <FONT size=2>regarding "attritioned" what dictionary are you referring to?</FONT>

    <FONT size=2></FONT> 

    <FONT size=2>- Websters online doesnt find it: </FONT><FONT size=2>http://www.m-w.com/cgi-bin/dictionary?book=Dictionary&va=attritioned</FONT>

    <FONT size=2>- dictionary.com doesnt find it: </FONT><FONT size=2>http://dictionary.reference.com/search?q=attritioned</FONT>

    <FONT size=2>- Microsoft Word 2003 doesnt recognize it either</FONT>

    <FONT size=2></FONT> 

    [pi]<FONT size=2> :P</FONT>

  • (cs)

    Sounds like the guy just read that very morning that in .Net, to throw an exception is very expensive, so he cut 12 exceptions down to just 1.

    But oh yea, he's a moron and didn't know wtf he was doing.

    (This post is [image] approved)

  • (cs) in reply to Ytram

    While I will forcefully edit code that assumes that everybody has changed their tab stop settings from the default of 8 to 4 then used tabs to indent all their code, I won't change code that has the obnoxious 'curly brace on a line by itself' style. But it is obnoxious.

    It's justified when the code inside the () for the while or if or whatever spans multiple lines. because otherwise the code for what's inside can scan and get mixed together with the code outside. But otherwise, it just introduces pointless whitespace.

    There is pointful whitespace that helps separate different functional groupings of code. And there's pointless whitespace which does nothing more than add a useless line that keeps me from seeing as much as i want to in my editor window. I can see no good reason to separate the code for an if, while, or for block from everything else when you have many other useful visual cues like whitespace. The code inside is obviously very related to the conditionals outside since its execution depends on their value.

  • (cs) in reply to Omnifarious

    *sigh* Oh, for the ability to edit posts. What I mean up there is:

    "many other useful visual cues like whitespace" should read "many other useful visual cues like indentation".

  • (cs)

    <font>This guy has some serious insights. ArgumentOutOfRangeException, ArgumentNullException, and ArgumentException are all ArgumentException's. By consolidating the exceptions, Jed has effectivly made this code general enough to be used across multiple platforms and projects.

    Seriously though, as </font>    JohnO mentioned, it's quite sad how people don't get fired for this. I feel your pain, Steve D.
    <font>
    </font>    [1] No, a<font>ttritioned is not a word. I looked it up.</font>

  • (cs) in reply to Djinn

    Djinn:

    [1] No, a<FONT size=+0>ttritioned is not a word. I looked it up.</FONT>

    Oh you won't find attritioned in any dictionary ... but it's a word!

  • Anonymous (unregistered) in reply to Djinn

    Djinn:
    <FONT size=+0>This guy has some serious insights. ArgumentOutOfRangeException, ArgumentNullException, and ArgumentException are all ArgumentException's. By consolidating the exceptions, Jed has effectivly made this code general enough to be used across multiple platforms and projects.

    Seriously though, as </FONT>    JohnO mentioned, it's quite sad how people don't get fired for this. I feel your pain, Steve D.
    <FONT size=+0>
    </FONT>    [1] No, a<FONT size=+0>ttritioned is not a word. I looked it up.</FONT>

     

    But attrition is!

  • (cs) in reply to Anonymous
    Anonymous:
    Djinn:
    [1] No, attritioned is not a word. I looked it up.
    But attrition is!

    And any noun can be verbed.

  • (cs) in reply to haveworld
    haveworld:
    seebs:
    It's worse than it looks. These tests can never all succeed. The expansionRate can never be: 1. Equal to 1. 2. Less than 1 or greater than 6. Thus, at least two of the possible invalid arguments are mutually exclusive, and this will never report an error. The entire snippet can be replaced with: /* Validate arguments */


    You can't say that becuase you don't know how many tests there are...


    I feel pretty sure that the number of test cases were 12 before the editor did his snipping, Alex can you confirm that?

    /Patric
  • cdn (unregistered)

    I'm guessing that Jed read somewhere that having multiple exit points in a function is bad, and reasoned that since exceptions cause a function to exit then having multiple exceptions must be bad.

  • (cs)
    Alex Papadimoulis:

    ...In addition to all 250+ opening curly braces being moved to the same line as the method/if/while statement...


    #ifdef seriously
    We all have our preferences when it comes to placing the curly braces and I hate as much as my fellow programmer to read code with them placed in the wrong spots. Whatever your preference is, don't change all of them using some smooth format tool because it will mess up diffing in your source control tool. When diffing two versions of a file to see what really have changed and you find 251 changes, one real one and 250 bogus ones. Also for reading purposes, just stick to the standard the original creator of the file had, even if it makes your heart bleed and eyes pop out. Best thing is of course to have a code standard which defines this and at code reviews check for these things. Even better would be if the IDE could display code formatted your way and save it the way your code standard defines. Sounds like a product idea, shame that developers don't pay for simple tools, they either just use them freely or code them themselves.
    #endif

    Jed sucks, changing test code without running them afterwards is just crazy, how could he expect to change the return values of test cases without affecting the tests?
  • (cs) in reply to Omnifarious
    Omnifarious:
    Anonymous:
    Djinn:
    [1] No, attritioned is not a word. I looked it up.
    But attrition is!

    And any noun can be verbed.

     

    That made me actually laugh out loud, or "LOL" as the kids are saying these days.

    Like on PointlessWasteOfTime's story about how to not get tsunami'd. Great stuff.

  • (cs)

    Attrition is the gradual weakening, destruction, or death of something.

    The verb form of "attrition" is "attrite"; to attrite something is to cause such weakening, destruction, or killing.

    The non-word 'attritioned' should therefore refer to something that has been weakened due to attrition.  The description 'the guy who not really a great programmer, and definitely not management material, yet who's been there so long that he's somehow attained a "senior" title' does not seem to describe a victim of attrition. 

    Perhaps the desired word is 'titular'?

  • (cs)

    Jed:  "I believe you have my stapler."

  • James Schend (unregistered) in reply to Manni
    Manni:
    Omnifarious:
    Anonymous:
    Djinn:
    [1] No, attritioned is not a word. I looked it up.
    But attrition is!

    And any noun can be verbed.

     

    That made me actually laugh out loud, or "LOL" as the kids are saying these days.

    Like on PointlessWasteOfTime's story about how to not get tsunami'd. Great stuff.



    I believe Calvin of Calvin and Hobbes said it best:

    Verbing weirds language.
  • (cs) in reply to theDrip
    theDrip:
    Jed:  "I believe you have my stapler."



    I had to fight not to actually laugh out loud.... you made my day.
  • Some Developer (unregistered)

    I once had a senior developer check my code out, format all the files so the the curley braces were on the same line, and checked it back in.   Said senior programmer is younger and less experienced than me, but has been with the my current company longer.   .

  • Anonymous (unregistered) in reply to Omnifarious
    Omnifarious:

    While I will forcefully edit code that assumes that everybody has changed their tab stop settings from the default of 8 to 4 then used tabs to indent all their code, I won't change code that has the obnoxious 'curly brace on a line by itself' style. But it is obnoxious.

    It's justified when the code inside the () for the while or if or whatever spans multiple lines. because otherwise the code for what's inside can scan and get mixed together with the code outside. But otherwise, it just introduces pointless whitespace.

    There is pointful whitespace that helps separate different functional groupings of code. And there's pointless whitespace which does nothing more than add a useless line that keeps me from seeing as much as i want to in my editor window. I can see no good reason to separate the code for an if, while, or for block from everything else when you have many other useful visual cues like whitespace. The code inside is obviously very related to the conditionals outside since its execution depends on their value.



    Having curly braces on a seperate line allows you to easily scan to verify bracers match correctly.  Yes, that's trivial, but so is your argument.

    That said, it's a design style.  It's like arguing about whether to use pre- or post- unary operators.  Stick with whatever your companies coventions are.
  • ComputerGuyCJ (unregistered) in reply to theDrip
    theDrip:
    Jed:  "I believe you have my stapler."

    "I'm gonna...burn the building down."
  • (cs) in reply to Omnifarious
    Omnifarious:
    Anonymous:
    Djinn:
    [1] No, attritioned is not a word. I looked it up.
    But attrition is!

    And any noun can be verbed.

    Ah, but may I suggest a less commonly used but perhaps more appropriate definition of attrition http://www.newadvent.org/cathen/02065a.htm

    Namely, turning away from sin for a less perfect motive than love of God [A], for example, fear of hell. In this case, that might mean keeping code away from Jed for fear of hell... [6]

  • (cs) in reply to Anonymous
    Anonymous:
    Omnifarious:

    While I will forcefully edit code that assumes that everybody has changed their tab stop settings from the default of 8 to 4 then used tabs to indent all their code, I won't change code that has the obnoxious 'curly brace on a line by itself' style. But it is obnoxious.

    It's justified when the code inside the () for the while or if or whatever spans multiple lines. because otherwise the code for what's inside can scan and get mixed together with the code outside. But otherwise, it just introduces pointless whitespace.

    There is pointful whitespace that helps separate different functional groupings of code. And there's pointless whitespace which does nothing more than add a useless line that keeps me from seeing as much as i want to in my editor window. I can see no good reason to separate the code for an if, while, or for block from everything else when you have many other useful visual cues like whitespace. The code inside is obviously very related to the conditionals outside since its execution depends on their value.



    Having curly braces on a seperate line allows you to easily scan to verify bracers match correctly.  Yes, that's trivial, but so is your argument.

    That said, it's a design style.  It's like arguing about whether to use pre- or post- unary operators.  Stick with whatever your companies coventions are.

    Let me get this straight, a guy takes a working validation routine, causes crap data to pass validation, makes the error messages meaningless, and people are busy having an rwar about curly brace lines? [:O]

    That's a WTF in of itself [H]. Now I see how WTF code makes it into production, all the developers who know better spend their day arguing with each other over curly brace styles and so the PM forbids all code reviews in order to let the janitors clean the blood off the floor quicker than it spills.

  • (cs) in reply to Anonymous
    Anonymous:
    Omnifarious:

    While I will forcefully edit code that assumes that everybody has changed their tab stop settings from the default of 8 to 4 then used tabs to indent all their code, I won't change code that has the obnoxious 'curly brace on a line by itself' style. But it is obnoxious.

    It's justified when the code inside the () for the while or if or whatever spans multiple lines. because otherwise the code for what's inside can scan and get mixed together with the code outside. But otherwise, it just introduces pointless whitespace.

    There is pointful whitespace that helps separate different functional groupings of code. And there's pointless whitespace which does nothing more than add a useless line that keeps me from seeing as much as i want to in my editor window. I can see no good reason to separate the code for an if, while, or for block from everything else when you have many other useful visual cues like whitespace. The code inside is obviously very related to the conditionals outside since its execution depends on their value.



    Having curly braces on a seperate line allows you to easily scan to verify bracers match correctly.  Yes, that's trivial, but so is your argument.

    That said, it's a design style.  It's like arguing about whether to use pre- or post- unary operators.  Stick with whatever your companies coventions are.


    Careful with that axe, Eugene.

    int a = 0;

    a = a++ + 1;
    System.out.println(a);

    a = ++a + 1;

    System.out.println(a);

    Run that in Java. Now, switch to C/C++ (changing variable printing accordingly), and run it against many different compilers/platforms.

  • (cs)

    Removing the curly brackets is really annoying.

    But giving too detailed error messages does not give your program a professional look, if i got an errormessage like this:

    ArgumentException: if expansionRate is 1, partCoefficient cannot possibly be 1
    (Ever had one like that?)
    It usually does not help the user since he/she does not know the internals of your programs.
    But it leaves the user whit a bad idea about your software, because it gives al kind of weird errormessages
    An errormessage like this:
    ArgumentException: Invalid Arguments.
    looks much better imho, and is much more userfriendly.
    For a developer using the code it is indeed not very practical.

    That just leaves a typo in the last line, which had to be something like
    if (invalidArgs > 0)

  • Russ (unregistered)

    Everyone loves made up words.  My favorite one that I made up is "ambigufy" - logic being:

    Clear -> Clarity and Clarify

    Ambiguous ->  Ambiguity and ____?  - Hence, ambigufy!

  • (cs) in reply to Anonymous
    Anonymous:

    It's like arguing about whether to use pre- or post- unary operators.


    Oh boy...them's fightin' words on this forum...

    -ds

  • (cs) in reply to Ytram
    Ytram:

    <!--StartFragment --> 

    Alex Papapapasomething:

    ...all 250+ opening curly braces being moved to the same line as the method/if/while statement...


    That's enough to piss me off.  I find that very unreadable.

    I'm willing to bet that Jed is a C-snob that thinks exceptions are stupid and that error codes are the only way to go.

    [image]



    Damn you and your pizza icon!
  • anon (unregistered)

    Steve clearly needs to fix his test cases.

    But seriously, chalk one up for the almighty "unit testing"!  Hurrah!



    Obligatory code review change....

    switch(invalidArgs)
    {
        default:
            if (IsTrue(invalidArgs == 12))
            {
                throw new ArgumentException("Invalid Arguments.");
            }
            break;
    }
  • grunting coder (unregistered)

    I'm a "programmer" who started off self-taught, developed some bad habits, and then had some schooling afterwards. I’m working in Java with a group of people who make me feel inadequate. I tremble in fear that something I’ve written when I was learning will show up on this site. When the regulars start getting into flamewars about optimization minutia, I am confused and angered.

    <!--[if !supportEmptyParas]--> <!--[endif]--><o:p></o:p>

    Yet I see an example like this and think “Wow, that’s really dumb. Really, really, dumb.”

    <!--[if !supportEmptyParas]--> <!--[endif]--><o:p></o:p>

    Then I can get back to my work with a spring in my step.

  • rhett (unregistered) in reply to koedoot

    koedoot:

    But giving too detailed error messages does not give your program a professional look, if i got an errormessage like this:

    ArgumentException: if expansionRate is 1, partCoefficient cannot possibly be 1

    (Ever had one like that?)
    It usually does not help the user since he/she does not know the internals of your programs.

    Who said that message is going to be displayed to the user?  Maybe the exception is going to be logged somewhere.  The top level UI should decide what to show the user.  The domain level objects should throw errors with full information.

  • (cs) in reply to anon

    anon:

    switch(invalidArgs)
    {
        default:
            if (IsTrue(invalidArgs == 12))
            {
                throw new ArgumentException("Invalid Arguments.");
            }
            break;
    }

    I think we determined yesterday that IsTrue is old and busted.

    Pizza's still cool though.

    [image]

  • (cs) in reply to Alex Papadimoulis

    [Y]

  • (cs) in reply to koedoot
    koedoot:
    Removing the curly brackets is really annoying.

    But giving too detailed error messages does not give your program a professional look, if i got an errormessage like this:
    ArgumentException: if expansionRate is 1, partCoefficient cannot possibly be 1
    (Ever had one like that?)
    It usually does not help the user since he/she does not know the internals of your programs.
    But it leaves the user whit a bad idea about your software, because it gives al kind of weird errormessages
    An errormessage like this:
    ArgumentException: Invalid Arguments.
    looks much better imho, and is much more userfriendly.
    For a developer using the code it is indeed not very practical.

    That just leaves a typo in the last line, which had to be something like
    if (invalidArgs > 0)


    Certainly your error messages shouldn't be spitting out variable names but what would be more helpful, telling you the name of the input you gave and the reason it is invalid or simply saying "Invalid arguments"? Let's suppose there are 10 arguments. Which ones were invalid and which ones were valid?

    Detail is nice.
  • Bob (unregistered) in reply to koedoot

    Anyone who just sends an "Invalid Arguments" error message to a dialog box for a user ought to be shot. I'ld rather have any of the daily WTF authors as my mentor than put up with someone who would do anything that amazingly pathetically stupid and evil. (They're often stupid beyond belief, rarely are they deliberately as malicious and spiteful as that.)

    Whether you use exceptions or error code is irrelevant - they're both messages from one block of code to another - they are never (under any circumstances) a way to pass error message strings to a dialog box.

    Users don't want to click a menu item and get a dialog box saying "ArgumentException: Invalid Arguments" because they're not arguing, they didn't write the code, and (most importantly) it doesn't tell them what to do about the problem or how to avoid it happening again.

    Yeah, error handling is hard - that's no excuse to just dump your exceptions on the users, especially if you want a "friendly" completely vague error message instead of at least telling the user what specific thing they entered that was wrong.

    Hell, the real WTF here is that Jed isn't the source of the most user-hostile suggestions in this thread.

  • MyTwoCents (unregistered) in reply to Omnifarious
    Omnifarious:

    While I will forcefully edit code that assumes that everybody has changed their tab stop settings from the default of 8 to 4 then used tabs to indent all their code, I won't change code that has the obnoxious 'curly brace on a line by itself' style. But it is obnoxious.

    It's justified when the code inside the () for the while or if or whatever spans multiple lines. because otherwise the code for what's inside can scan ... /* snip */ ... like whitespace. The code inside is obviously very related to the conditionals outside since its execution depends on their value.



    SHUT TF UP! If you feel that strongly get/write yourself a code formatter != rocket_science
  • jose cuervo (unregistered) in reply to Russ
    Anonymous:

    Everyone loves made up words.  My favorite one that I made up is "ambigufy" - logic being:

    Clear -> Clarity and Clarify

    Ambiguous ->  Ambiguity and ____?  - Hence, ambigufy!



    You mean ambiguate?  Not really a word, but disambiguate is.
  • (cs)
    Alex Papadimoulis:

    In addition to all 250+ opening curly braces being moved to the same line as the method/if/while statement, the argument validation was just no longer working.

    The "before" version ...

    Neither version has any curly braces that I can see. This is bad form. Every if & while should use curly braces to enclose the block, even if there's only zero or one statement inside it. The extra braces are a small price to pay for consistency & clarity, & the decreased risk when someone comes along & adds another statement to the block.

    <FONT size=1>(Just-cuz-the-language-lets-you-do-it-doesn't-mean-it's-a-good-idea dept.)</FONT>

  • (cs) in reply to Djinn

    Omnifarious wrote:

    "for reading purposes, just stick to the standard the original creator of the file had"

    What if the original author's style/formatting has been corrupted by 3 years of maintenance by tens of other developers who inserted new code or rewrote bits of old code in their own styles?  I'd say that once you get to this point, a style overhaul might not be a bad thing, just to make it uniform.


    "Best thing is of course to have a code standard which defines this and at code reviews check for these things"

    This has its benefits as well as its drawbacks.

Leave a comment on “Argument About Argument Validation”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article