[FIXED] Getting error while creating an Enum with each value pointing to a list of String

Issue

public enum Test implements IsSerializable {

    FOOD("FOOD", getFoodItems()),//---Getting Error here
    HOTEL("HOTEL", getHotels());//---Getting Error here

    private String name;
    static final List<String> categories = new LinkedList<String>();

    Test(String name) {
        this.name = name;
    }
    public List<String> getCategories() {
       return this.categories;
    }


    private  List<String> getFoodItems(){
       categories.add("Food item1");
       categories.add("Food item2");
       return categories;
    }
    private  List<String> getHotels(){
       categories.add("Hotel 1");
       categories.add("Hotel 2");
       return categories;
    }
}

I am getting error while creating this Enum. I am new to this type Enum . Can anyone help to make this work?

Solution

3 main things:

1. getFoodItems() and getHotels() should be static.

The methods need the existing enum to exist and do not even use anything in the enum.

2. don't declare categories as a static attribute..

You use the same categories object(or references to the same object) because it is static. Remove the static keyword before it in order to make it a member attribute. Each object should have it's own list and this is not possible if it is static.

3. create the List inside the method and give it to the constructor

You call the constructor with 2 parameters: the name and the List but the constructor does only accept the name. Create the categories object inside the methand and return it to the constructor.

3 additional improvements from @Holger (see the comments)

4. Think twice before handing out references to mutable lists.

If you pass a mutable list to a method, that method can change the list which could lead to bugs that may be difficult to find.

5. The name is entirely obsolete as there is no getter for it and its value matches the intrinsic name() anyway.

You may want to add it in any case but enumerations already have a method name() that returns the name of the enum.

6. Don’t use LinkedList when you don’t need its special features (in other words, never use it), but rather, use an ArrayList or even better, List.of(item1, item2), to create an immutable list.

ArrayList is more performant that LinkedList in general because it is based on an array and LinkedList is a linked list(as the name says) that has to create an object for every element containing reference to it's neighbours. That is an advantage if you need to add elements in the middle (or the start) of the list or if you need Queue or Dequeue functionality.

public enum Test implements IsSerializable {
    FOOD("FOOD",getFoodItems()),//---Getting Error here.
    HOTEL("HOTEL",getHotels());//---Getting Error here
    private String name;
    private final List<String> categories;
    private Test(String name,List<String> categories) {
        this.name = name;
        this.categories=categories;
    }
    public List<String> getCategories() {
        return this.categories;
    }
    private static List<String> getFoodItems(){
        List<String> categories = new LinkedList<>();
        categories.add("Food item1");
        categories.add("Food item2");
        return categories;
    }
    private static List<String> getHotels(){
        List<String> categories = new ArrayList<>();
        categories.add("Hotel 1");
        categories.add("Hotel 2");
        return categories;
     }
}

​

[Edit] Please also take note of the answer from boot-and-bottet

Answered By - dan1st
Answer Checked By - Marie Seifert (JavaFixing Admin)