1

I want to convert an ArrayList<String> to Set<ScopeItem> with Java streams.

ScopeItem is a enum;
items is an ArrayList<String>;

Set<ScopeItem> scopeItems = items.stream()
                    .map(scopeString -> ScopeItem.valueOf(scopeString))
                    .filter(Objects::nonNull)
                    .collect(Collectors.toSet());

On a string that isn't in the enum this throws the following:

java.lang.IllegalArgumentException: No enum const...

Ideally, I would like to skip past any Strings that don't match.

I think maybe a using flatmap? Any ideas how to do it?

Abraham
  • 8,525
  • 5
  • 47
  • 53
user7862512
  • 273
  • 1
  • 9
  • Can your list contain strings which don't *exactly* match your enum, and what is the delta between them? – Makoto Jan 29 '18 at 18:42
  • @Makoto yes, this is the problem. I want it to be able to handle non-matching strings (by simply ignoring them). I'm not sure what you mean by the delta between them though? It should match exactly, case sensitive, etc – user7862512 Jan 29 '18 at 18:43
  • Assuming you can change implementation of `ScopeItem`, I suggest [adding a contains() method to your enumeration](https://stackoverflow.com/questions/4936819/java-check-if-enum-contains-a-given-string) – M. Prokhorov Jan 29 '18 at 18:43
  • What I'm saying is you have an incomplete error message. If you're getting a value of "foobasdfkj1236798723" for an enum and you obviously don't support that, this begs the question of where those values came from in the first place. – Makoto Jan 29 '18 at 18:44
  • 1
    Also, depending on what your `ScopeItem`s used for, you actually might want it to "blow up" as soon as possible, as ignoring silently values might be the source of really subtle bugs. So, maybe just convert default exception to something with more useful message. – M. Prokhorov Jan 29 '18 at 18:52

6 Answers6

3

You could add the following method to your ScopeItem:

public static ScopeItem valueOfOrNull(String name) {
    try {
        return valueOf(name);
    } catch (IllegalArgumentException e) {
        // no such element
        return null;
    }
}

and use that to map your enum values:

.map(scopeString -> ScopeItem.valueOfOrNull(scopeString))

Subsequent .filter() on non-null values (which you already have) will filter-out those nulls that correspond to non-matching strings.

Makoto
  • 104,088
  • 27
  • 192
  • 230
Roman Puchkovskiy
  • 11,415
  • 5
  • 36
  • 72
  • 1
    The more I think about it, the less I'm enamored about having exceptions as conditions here. – Makoto Jan 29 '18 at 19:08
1

You can put a try-catch inside your map to return null instead of throwing an exception:

Set<ScopeItem> scopeItems = items.stream()
    .map(scopeString ->
        {
           try
           {
              return ScopeItem.valueOf(scopeString);
           }
           catch (IllegalArgumentException e)
           {
              return null;
           }
        })
    .filter(Objects::nonNull)
    .collect(Collectors.toSet());

You could also use filter beforehand to check whether the array of values contains the string you're looking for:

Set<ScopeItem> scopeItems = items.stream()
    .filter(scopeString -> Arrays.stream(ScopeItem.values())
                               .anyMatch(scopeItem -> scopeItem.name().equals(scopeString)))
    .map(ScopeItem::valueOf)
    .collect(Collectors.toSet());
Bernhard Barker
  • 54,589
  • 14
  • 104
  • 138
1

Unlike others, I won't recommend using exceptions, as I feel they should be used for exceptional situations, and not for something that will likely to occur. A simple solution, is to have a static set of acceptable strings, and simply check, if a string you want to use valueOf with is in said set.

package test;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class Test {

    public static enum ScopeItem {
        ScopeA,
        ScopeB;

        private static Set<String> castableStrings;

        static {
            castableStrings = new HashSet<>();
            for (ScopeItem i : ScopeItem.values()) {
                castableStrings.add(i.name());
            }
        }

        public static boolean acceptable(String s) {
            return castableStrings.contains(s);
        }
    }

    public static void main(String[] args) {

       List<String> items = Arrays.asList("ScopeA", "RandomString", "ScopeB");
       Set<ScopeItem> scopeItems = items.stream()
               .filter(ScopeItem::acceptable)
               .map(ScopeItem::valueOf)
               .collect(Collectors.toSet());

       System.out.println(scopeItems.size());
    }

}
Coderino Javarino
  • 2,819
  • 4
  • 21
  • 43
  • +1 for mentioning Exceptions should *only* be used for exceptional behavior. Exceptions should not be used as conditional flow when there are easy alternatives (this answer, and a couple others here). – Ironcache Jan 29 '18 at 19:56
  • @Makoto True, why people even bother with getters/setters, if that's more work than the truly required public class fields. – Coderino Javarino Jan 29 '18 at 20:36
  • 1
    @CoderinoJavarino That comparison is kind of apples to oranges. In this case, the boilerplate code is a static look-up set (which is in the name of efficiency). Getter and setter boilerplate code safeguards against fundamentally dangerous scenarios down the road (for example, synchronizing access to variables which are, later on in development, accessed by multiple threads). Efficiency *may* be a factor (in which case the author should make considerations for your solution), but safeguarding access should *always* be a concern. – Ironcache Jan 29 '18 at 20:58
1

There's a more elegant approach here. You don't have to add any new fields to your enum; you can simply run a stream against it as well and determine if there's any matches in your collection.

The below code assumes an enum declaration of:

enum F {
    A, B, C, D, E
}

and looks as thus:

List<String> bad = Arrays.asList("A", "a", "B", "b", "C", "c");

final Set<F> collect = bad.stream()
                                .filter(e -> Arrays.stream(F.values())
                                                     .map(F::name)
                                                     .anyMatch(m -> Objects.equals(e, m)))
                                .map(F::valueOf)
                                .collect(Collectors.toSet());

Two parts to pay attention to here:

  • We do the internal filter on the values of our enum, and map that to a String through F::name.
  • We determine if there's any match on the elements of our base collection with the elements of our enum in a null-safe way (Objects.equals does The Right Thing™ with nulls here)
Makoto
  • 104,088
  • 27
  • 192
  • 230
0

You could use Apache Common's commons-lang3 EnumUtils.getEnum() instead of valueOf(). This returns null if there is no matching enum entry (which you can then filter exactly as you have in your code).

Ironcache
  • 1,719
  • 21
  • 33
Simon Berthiaume
  • 643
  • 4
  • 11
0

The easiest and clearest method would be to filter your Enum's values() method on items.contains():

Set<ScopeItem> enumVals = Arrays.stream(ScopeItem.values())
        .filter(e -> items.contains(e.name()))
        .collect(Collectors.toSet());

No added functionality just to get the stream to do what you want, and it is obvious what this does at a glance.

Ironcache
  • 1,719
  • 21
  • 33
  • Problem is, you'll be iterating through whole `items` list for each missing enum value, meaning that depending on the `items` size, this solution can be few times slower than the other ones. – Coderino Javarino Jan 29 '18 at 19:17
  • @CoderinoJavarino Both `Enum.values()` and `items` are backed by arrays. How is iterating over one and checking if the other contains each entry any slower than the reverse? It's `O(n * m)` in either direction, where `n` and `m` are the sizes of the respective arrays. – Ironcache Jan 29 '18 at 19:24
  • Also note that, if efficiency is a concern, Exception handling is *extremely* slow relative to array access/iteration, and this solution will likely outperform most of the others listed here by virtue of that alone. – Ironcache Jan 29 '18 at 19:31
  • Hmm that's true, I guess I wrote that comment too quickly. I suppose an argument could be made in favor of iterating items, that you can create a constant time lookup for the enum (e.g. my answer) since enum stay the same, while items can be different with each code snippet execution. You're right about exceptions, not a fan of it. – Coderino Javarino Jan 29 '18 at 19:52