The Daily WTF: Curious Perversions in Information Technology
Welcome to TDWTF Forums Sign in | Join | Help
in Search

Needs more IllegalArgumentException

Last post 11-13-2012 3:32 PM by _leonardo_. 7 replies.
Page 1 of 1 (8 items)
Sort Posts: Previous Next
  • 11-09-2012 8:32 AM

    • grkvlt
    • Not Ranked
    • Joined on 04-24-2006
    • Edinburgh
    • Posts 14

    Needs more IllegalArgumentException

    I found this in some validation code. It looks like there has been a failed attempt to follow the pattern that was used for all the other validation methods, coupled with a misunderstanding about what exceptions are for:
        public String validateThing(@Nullable String thing, String objectName, String fieldName) {
            if (thing != null) { // thing can be null.
                try {
                    if (!thing.equalsIgnoreCase(Thing.CONSTANT))
                        throw new IllegalArgumentException();
                } catch (IllegalArgumentException e) {
                    throw new IllegalArgumentException(messages.getMessage("validation.thing", new Object[]{objectName, fieldName}, locale.get()));
                }
            }
            return null;
        }
    
    The return null at the end is particularly fun, since these methods are meant to be used like this:
        String thing = validateThing(input, "object", "field");
    
    do not fold, bend, spindle, or mutilate
  • 11-09-2012 2:17 PM In reply to

    Re: Needs more IllegalArgumentException

    grkvlt:
    I found this in some validation code. It looks like there has been a failed coder.

    FTFY

    TRWTF is people who've never heard of Blinkenlights. The Jargon File should be required reading.
  • 11-09-2012 2:21 PM In reply to

    Re: Needs more IllegalArgumentException

    Is it a feature that it can only return null?

    I spend most of my life pressing buttons to make the pattern of lights change however I want.
  • 11-09-2012 5:03 PM In reply to

    Re: Needs more IllegalArgumentException

    actually: yes!  This method should have a void return type, and should simply throw the exception or complete normally.  A method named "validateFoo" should not return a value.

    String validatedFoo = validateFoo(foo);//why reassign your output into your input?  was the original value good or did it change??

     

    Now, if your method were named "cleanseFoo", or "formatFoo", then i'd expect to see you use it like: 

    String cleanFoo = cleanseFoo(foo); //the name of this method accurately reflects what it's doing.

    Filed under:
  • 11-09-2012 7:19 PM In reply to

    • ekolis
    • Top 75 Contributor
    • Joined on 01-09-2008
    • Cincinnati, OH, USA
    • Posts 591

    Re: Needs more IllegalArgumentException

    _leonardo_:
    actually: yes!  This method should have a void return type, and should simply throw the exception or complete normally.  A method named "validateFoo" should not return a value.
     

    It could return a value, if the value is an error message or list of error messages to display to the user. I've actually done this a few times; not sure if it's a bad idea, but it got the job done! I'm guessing OP's code used to return an error message, but someone told whoever wrote it to make it throw exceptions instead, so he did so, but never actually changed the return type...

    I'm Spark Mandrill, and I'll... hey... can I... what, it BOUNCES?... 'kay, I'm splodin' now.
  • 11-09-2012 8:51 PM In reply to

    • grkvlt
    • Not Ranked
    • Joined on 04-24-2006
    • Edinburgh
    • Posts 14

    Re: Needs more IllegalArgumentException

    ekolis:
    I'm guessing OP's code used to return an error message
    Actually, no. The code here was originally meant to be called similarly to the Guava library Preconditions.checkNotNull method. So, something like this:
    String checked = Preconditions.checkNotNull(argv[0], "the input was null"); // throws NullPointerException
    String thing = validator.validateThing(argv[1], "thing", "thingy"); // throws IllegalArgumentException
    
    But, the other Guava precondition check methods in that class don't return their argument, and in the code that calls our validators the return value is always discarded. So, I think I should just make them void return types, like _leonardo_ suggests...

    My WTF, perhaps?

    do not fold, bend, spindle, or mutilate
  • 11-13-2012 8:37 AM In reply to

    Re: Needs more IllegalArgumentException

     I'm not sure, more than half of the methods in the page you linked do return their argument (probably because it allows putting them directly in a method call, e. g. obj.Method(checkNotNull(arg))).

    I guess it's just a matter of convenience, and code conciseness (putting validation on separate lines is cumbersome).

  • 11-13-2012 3:32 PM In reply to

    Re: Needs more IllegalArgumentException

    Medinoc:

    ...obj.Method(checkNotNull(arg)))....

     

     even better... thanks!

     

    Filed under:
Page 1 of 1 (8 items)
Powered by Community Server (Non-Commercial Edition), by Telligent Systems