1

I have the following enum, but i want to iterate over the second argument only without having to iterate over the enum member that are outside of this category, so if i have a message:"Message" and category "Category" i can specify the category as argument too in the method isMessageInGroup without having to iterate over those with another category like MESSAGE_3:"Another Category"

Is there a neat way to do it and save some iteration time? probably not even with a great deal of values this will affect performance noticeably, but i want to know if it possible. Searched for a bit but hard to find this specific question.

The Enum below does charge for messages by category but i want to know if i can avoid iterating over those outside the category wanted

public enum MessagesEnum {
    MESSAGE_1("Message", "Category"),
    MESSAGE_2("Another Message", "Category"),
    MESSAGE_3("Odd Message", "Another Category");

    private final String message;
    private final String category;

    SabreErrorMessages(String message, String errorCategory) {
        this.message = message;
        this.category = category;
    }

    public String getMessage() {
        return message;
    }

    public String getCategory() {
        return category;
    }

    public static boolean isMessageInGroup(String message){
        for(MessagesEnum message : MessagesEnum.values()) {
            if(message.contains(message.getMessage()) && message.getCategory().equals("Category")) {
                return true;
            }
        }
        return false;
    }
}
Ringo
  • 850
  • 9
  • 24
  • 2
    Just have a static final field with a `Map>` that builds up a `Map` when the class is loaded. – Louis Wasserman Jun 20 '16 at 21:59
  • 2
    @LouisWasserman is right, but perhaps with the caveat that it should maybe be a `Map>` since then you can check `contains` without iterating anything at all. – Trevor Freeman Jun 20 '16 at 22:05
  • Got it, should've occurred to me with some creativity, there is always a smart way. Thank you both – Ringo Jun 20 '16 at 22:50

1 Answers1

5

As the comments have said, an out-of-the-box enum won't be most efficient for this because you will have to use an iterator. HashMap, however, provides O(1) lookup on average and will be much faster.

public enum Messages {

    MESSAGE_1("Message", "Category"),
    MESSAGE_2("Another Message", "Category"),
    MESSAGE_3("Odd Message", "Another Category");

    private static final Map<String, Set<String>> map = new HashMap<>();
    static {
        for (Messages m : Messages.values()) {
            map.computeIfAbsent(m.category, s -> new HashSet<>()).add(m.message);
        }
    }

    private final String message, category;

    private Messages(String message, String category) {
        this.message = message;
        this.category = category;
    }

    public String getMessage() { return message; }
    public String getCategory() { return category; }

    public static boolean isMessageInGroup(String message){
        // use `getOrDefault` if `get` could return null!!
        return map.get("Category").contains(message);
    }
}

Ideone Demo

Edit: Should you choose to implement a method like messagesInGroup, the safest way would be to implement it using an unmodifiable Set so as to protect the integrity of the enum's internals.

public static Set<String> messagesInGroup(String category) {
    return Collections.unmodifiableSet(
        map.getOrDefault(category, Collections.emptySet())
    );
}
4castle
  • 32,613
  • 11
  • 69
  • 106
  • 1
    There's no particular need to change it from being an enum. Enums work well with this solution. – Paul Hicks Jun 20 '16 at 22:40
  • 1
    @PaulHicks Yes, I was thinking about that. It's fixed now. I had to reference [this question](http://stackoverflow.com/q/443980/5743988) to figure out how to do it. – 4castle Jun 20 '16 at 22:45
  • 2
    That's it. Though I doubt OP actually wants an `isMessageInGroup` method, that was just his attempt at a solution. Probably a more useful method would be `Collection MessagesInGroup(string category) { return map.get(category); }` which allows app code to do what it needs. – Paul Hicks Jun 20 '16 at 22:49
  • @PaulHicks Yes Paul, my solution ultimately should work for any category. I believe this is the most elegant way to do it. Thanks! – Ringo Jun 20 '16 at 22:53
  • @LuisDurazo Notice that I used `computeIfAbsent` which is a Java 8 feature of `Map`. If you aren't using Java 8, you will need to replace that with what you have available. – 4castle Jun 20 '16 at 22:54
  • @4castle yes, i'm acquainted with the java 8 features, didn't know of the existence of `computeIfAbsent` will read about it before using it as is, method name is very self explanatory though, just curious about how is implemented. – Ringo Jun 20 '16 at 22:56
  • 1
    Oh good. In that case you might also want to use `getOrDefault` with the suggestion from @PaulHicks so that you don't return `null`. I didn't use `getOrDefault` because I knew it wouldn't return `null`. (Be careful using the suggestion from Paul though, because it would be possible for the calling scope to modify the `Collection` and ruin the integrity of the Enum, so you should wrap it with `Collections.unmodifiableSet()`) – 4castle Jun 20 '16 at 23:01
  • Does `Collections.emptySet()` work where you currently have `new HashSet<>()`? – Paul Hicks Jun 21 '16 at 00:04
  • @PaulHicks Yes, I'm going to update because that would actually be preferable since it returns a singleton (and therefore saves memory and speed instead of creating a new object). It can't be used in `computeIfAbsent` though because the `Set` it returns is immutable. – 4castle Jun 21 '16 at 00:11