Replace Enums.valueOfFunction() with a bidirectional Converter

[gissuebot]

Original issue created by SeanPFloyd on 2011-04-14 at 04:32 PM

---

Today I came across

Enums.valueOfFunction(enumClass)

after implementing the same functionality myself (as usual :-))
But one of its features can also be seen as a bug:

Invalid elements return null instead of throwing an IllegalArgumentException while I would like to rely on the IllegalArgumentException as Validation device.

My scenario is this:

A DB contains a comma-delimited String, and I want to create an EnumSet of the Enum items from this String. Currently, what I do is this:

return Sets.newEnumSet(
    Iterables.transform(
        Splitter.on(',').omitEmptyStrings().split(enumString),
        Enums.valueOfFunction(enumClass)
    ),
    enumClass
);

Now, if the String contains an invalid entry, Enums.valueOfFunction(enumClass) returns null, and EnumSet.add() throws a NPE, which is less than satisfactory when it comes to detecting what went wrong.

Therefore I would like to have an overloaded version, something like

Enums.valueOfFunction(Class enumClass, boolean swallowException)

where I can turn this behavior off.

[gissuebot]

Original comment posted by gscerbak on 2011-04-14 at 07:53 PM

---

IMHO better would be alternative method without the boolean parameter, something like Enums.checkedValueOfFunction().

The boolean parameter switching between two behaviours is a smell.

[gissuebot]

Original comment posted by SeanPFloyd on 2011-04-15 at 07:23 AM

---

Sure, I'd be happy with that as well :-)

[gissuebot]

Original comment posted by ray.j.greenwell on 2011-04-16 at 01:30 AM

---

Perhaps a general-purpose null-intolerant pass-through function?

public static <F, T> Function<F, T> nullIntolerant (final Function<F, T> orig)
{
    return new Function<F, T>() {
        public T apply (F in) {
             T result = orig.apply(in);
              if (result == null) {
                  throw new NullPointerException("Input resulted in null: " + in);
              }
              return result;
        }
    };
}

Then your code is the same except for..

• Enums.valueOfFunction(enumClass)
• Functions.nullIntolerant(Enums.valueOfFunction(enumClass))

[gissuebot]

Original comment posted by neveue on 2011-04-16 at 03:07 PM

---

The thing is, the current Enums.valueOfFunction() is already null intolerant. It returns null when the input is a non-null, unknown enum name. Here is the current behavior:

public enum Job {
    CEO, DEVELOPER, DESIGNER
}

public class EnumsTest {
    private Function<String, Job> valueOfFunction = Enums.valueOfFunction(Job.class);

@Test(expected = NullPointerException.class)
public void testNull() {
    valueOfFunction.apply(null);
}

@Test
public void testOk() {
    assertEquals(valueOfFunction.apply("CEO"), Job.CEO);
}

@Test
public void testIllegal() {
    assertEquals(valueOfFunction.apply("UNKNOWN"), null);
}

}

I don't like the idea of swallowing IllegalArgumentExceptions, but I guess one could work around that in some cases using filtering:

Iterables.filter(Iterables.transform(jobNames, Enums.valueOfFunction(Job.class)), Predicates.notNull());

or, if you want to fail fast:

Iterable<Job> jobs = Iterables.transform(jobNames, Enums.valueOfFunction(Job.class));
Preconditions.checkState(Iterables.all(jobs, Predicates.notNull()), "Some unknown jobs");

[gissuebot]

Original comment posted by SeanPFloyd on 2011-04-18 at 07:45 AM

---

ray: I like the nullIntolerant() function, but it wouldn't help in this case, because again, I wouldn't get an error message telling me which input was invalid

nev...: I think we have a misunderstanding here. valueOfFunction() does swallo IllegalArgumentException and I am asking for a version that doesn't, not the other way around

[gissuebot]

Original comment posted by neveue on 2011-04-18 at 08:25 AM

---

I understand that the real problem is that illegal inputs are silently swallowed, and I definitely agree that an alternative would be interesting.

I was merely pointing out that the function is already null intolerant: it throws a NPE when the input is null (which is a good thing, IMO).

Now, re-reading Ray's suggestion, I see that his goal was not to make the valueOfFunction null-intolerant (it already is), but to fail on illegal inputs: the valueOfFunction returns "null" on illegal inputs, and the nullIntolerant wrapper then fails. The problem is, as you said, that it would not tell you which input was invalid.

[gissuebot]

Original comment posted by amertum on 2011-04-18 at 09:34 AM

---

I also think that function which convert to an enum should throws an exception when handling unknown string enum value because as enum are finite enumeration, we should fail-fast when using it. I think the java.lang.EnumConstantNotPresentException is a good candidate as it is made for this purpose. Moreover, as dev, if we really want to convert unknown string enum value, I'd prefer default value than null.

ie : Enums.valueOfFunction(Class<T> enumClass, T defaultValue);

[gissuebot]

Original comment posted by ray.j.greenwell on 2011-04-18 at 07:15 PM

---

My suggested function does actually include the null-causing input in the exception.

[gissuebot]

Original comment posted by kevinb9n on 2011-04-19 at 02:09 AM

---

When we release Converter, we'll be changing this to a Converter from a Function. Converters are bidirectional and will never convert any non-null value to null; an exception will have to be thrown. So this should be what you want.

[gissuebot]

Original comment posted by ray.j.greenwell on 2011-04-19 at 07:57 PM

---

That would be great. A String <-> Enum Converter is what we all want, and would also take care of issue 435.

[gissuebot]

Original comment posted by [email protected] on 2011-07-13 at 07:34 PM

---

(No comment entered for this change.)

---

Status: Accepted
Labels: Type-Enhancement

[gissuebot]

Original comment posted by [email protected] on 2011-07-18 at 03:51 PM

---

Issue #435 has been merged into this issue.

[gissuebot]

Original comment posted by [email protected] on 2011-07-18 at 03:53 PM

---

(No comment entered for this change.)

[gissuebot]

Original comment posted by [email protected] on 2011-12-10 at 04:05 PM

---

(No comment entered for this change.)

---

Labels: Package-Base

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-16 at 07:07 PM

---

(No comment entered for this change.)

---

Blocked On: #177

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-16 at 07:07 PM

---

We intend to release this when we release Converter.

[gissuebot]

Original comment posted by [email protected] on 2012-05-30 at 07:43 PM

---

(No comment entered for this change.)

---

Labels: -Type-Enhancement, Type-Addition
Blocked On: -#177, #177

[gissuebot]

Original comment posted by [email protected] on 2013-12-19 at 07:01 PM

---

75fda2a

---

Status: Fixed
Labels: Milestone-Release16
Blocked On: -#177, #177