Converting a String to Enum - the long way



  • In Java, you can convert a String to an Enum like this:

     

    public enum Xxx {
    A, B;
    public Xxx getValue(String s) { // Pass: "A" and get back Xxx.A
        return valueOf(s);
    }
    }

    ...and it's done this way all over our application. However, our most senior employee-developer did this:

     

    public class EnumXxxConversionUtil {
    public Xxx convertStringToXXX(String s) { // the Constants.VALUE*'s are strings
    if (Constants.VALUE1.equalsIgnoreCase(s)) return Xxx.VALUE1;
    if (Constants.VALUE2.equalsIgnoreCase(s)) return Xxx.VALUE2;
    ...
    if (Constants.VALUE47.equalsIgnoreCase(s)) return Xxx.VALUE47;
    return null;
    }
    }

    ...and repeated this pattern for about 30 different large enums.



  • To clarify, this genius replaced the former pattern with the latter - because the input strings weren't getting converted to the enum - exceptions were getting thrown. 

    It never occurred to this guy to check his input values.

    But that didn't stop him from checking in about 3000 LOC worth of pointless conversion utilities...



  • return null;

    Haha! Lovely! A null Enum!

    return valueOf(s);

    You can save yourself a lot of trouble if you do valueOf(s.toUpperCase()) and this is the perfect example.



  • Sorry, .Net guy here, but wouldn't toStringing this biotch get you the string representation of the enum?  This seems very convoluted...



  • @C-Octothorpe said:

    Sorry, .Net guy here, but wouldn't toStringing this biotch get you the string representation of the enum?  This seems very convoluted...

     

    If you have some enum X, then X.toString() should give you the string version of the enum literal, but in Java, by convention, you're supposed to use X.name() which does exactly the same thing.

    The point was that Java enums give you an implied built-in method: valueOf(String) to do the lookup and conversion for you; They also give you: values() which gives a list of string literals representing all the values, so at worst case, you can loop, without hard coding the values.



  • @snoofle said:

    They also give you: values() which gives a list of string literals representing all the values, so at worst case, you can loop, without hard coding the values.

    values() doesn't give you the string literals, but an array of all the enum values. Of course, you could always toString() or name() the values in the array, but yeah, either way, cascades of if statements are worse than failure.



  • built-in method: valueOf(String)

    Which you cannot override, will never return a null (because an enum can't be null) but throws an invalid argument exception.



  •  Xyro's correction: quite correct; my fault for attempting to quote from memory while in the middle of attempting to rework a query with 11 "union all", four "union", two "minus" and a couple of "with".... (turns out the whole fucking thing is never used and I reduced it to a null-op, which cascaded to a whole hierarchy of client-thru-server calls to disappear, for a total of 6500 LOC).


  • Garbage Person

    @snoofle said:

     Xyro's correction: quite correct; my fault for attempting to quote from memory while in the middle of attempting to rework a query with 11 "union all", four "union", two "minus" and a couple of "with".... (turns out the whole fucking thing is never used and I reduced it to a null-op, which cascaded to a whole hierarchy of client-thru-server calls to disappear, for a total of 6500 LOC).

    Tomorrow in your inbox shall be that thing as a feature request.


  • @ubersoldat said:

    built-in method: valueOf(String)
    Which you cannot override, will never return a null (because an enum can't be null) but throws an invalid argument exception.
    Which is why .Net finally gave us:<FONT face=Consolas size=2><FONT face=Consolas size=2></FONT></FONT><FONT face=Consolas size=2><FONT face=Consolas size=2>

    System.</FONT></FONT><FONT face=Consolas color=#2b91af size=2><FONT face=Consolas color=#2b91af size=2><FONT face=Consolas color=#2b91af size=2>Enum</FONT></FONT></FONT><FONT face=Consolas size=2><FONT face=Consolas size=2>.TryParse</FONT></FONT>

    This way, you can try to convert the string to its enumeration type without exception throwing.



  • @snoofle said:

     Xyro's correction: quite correct; my fault for attempting to quote from memory while in the middle of attempting to rework a query with 11 "union all", four "union", two "minus" and a couple of "with".... (turns out the whole fucking thing is never used and I reduced it to a null-op, which cascaded to a whole hierarchy of client-thru-server calls to disappear, for a total of 6500 LOC).

    Isn't that the best feeling?

    I did some profiling, and found that in all my test calls, an exception was being thrown and caught one layer up.  Turns out there was a "feature" that got "rolled back"... except out of our code.  I removed everything about that feature, and removed probably 500-1000 LOC.


Log in to reply