ArrayList of enums with custom attribute

Question

I have enum class that describes possible tickets types and have custom atribute to keep ticketId. When I try to add some tickets to ArrayList it makes all tickets of type X have the same ticketId. Why is that and what's more important how can I solve it?

Simplified enum class:

public enum Ticket {
    FirstClass(0),
    PremiumClass(1),
    EconomyClass(2);

    private int elementId;
    private Long ticketId;

    Ticket(int elementId) {
        this.elementId=elementId;
    }

    public Long getTicketId() {
        return ticketId;
    }

    public void setTicketId(Long ticketId) {
        this.ticketId = ticketId;
    }
}

Simplified method:

public void myMethod() {
    ArrayList<Ticket> tickets = new ArrayList<>();
    Ticket ticket = Ticket.FirstClass;
    ticket.setTicketId(1L);
    tickets.add(ticket);
    ticket = Ticket.FirstClass;
    ticket.setTicketId(2L);
    tickets.add(ticket);
}

Answer 1

It is happening because there is only one instance for every enum constant. Calling Ticket.FirstClass will fetch the same instance every time. So you are adding the same object to the list twice.

Answer 2

There is really only one instance of Ticket.FirstClass, and there will only ever be one instance.

What you really have isn't a Ticket, but a TicketType. You should have a separate class for Ticket.

Answer 3

Don't use enums for dynamic values,see sonar's "enum" fields should not be publicly mutable

enums are generally thought of as constant, but an enum with a public field or public setter is not only non-constant, but also vulnerable to malicious code.

See example of making enum dynamic by making it implement an interface

Answer 4

FirstClass is an instance of Ticket so when you're calling it, it's always the same one, you take the reference to the same object.

What you would need is a model with a Ticket and a TicketType

enum TicketType {
    FirstClass,
    PremiumClass,
    EconomyClass
}

class Ticket {
    private TicketType type;
    private Long ticketId;

    Ticket(TicketType type, long ticketId) {
        this.type = type;
        this.ticketId = ticketId;
    }
}
// ---------------------------------------------------------
// And
public void myMethod() {
    ArrayList<Ticket> tickets = new ArrayList<>();

    Ticket ticket = new Ticket(TicketType.FirstClass, 1L);
    tickets.add(ticket);

    ticket = new Ticket(TicketType.FirstClass, 2L);
    tickets.add(ticket);
}

Answer 5

Nothing different from other answers, just to add some colors. When you obtain firstClass you actually are getting same object, and adding that again.

Answer 6

Each enum value (you have got three) exists only once and each time you are using one of them you are just reusing one of those three values by creating a reference, there are no copies.

This means if you change the ticketId on the FirstClass value, it is changed wherever that value is referenced.

It seems you want to model a little ticket system. Each object having an own identity should be modeled as a class which has a property for the type and the id:

public class Ticket {
    private Long ticketId;
    private TicketType type;

    public Long getTicketId() {
        return ticketId;
    }

    public void setTicketId(Long ticketId) {
        this.ticketId = ticketId;
    }

    public TicketType getType() {
        return type;
    }

    public void setType(TicketType type) {
        this.type = type;
    }
}

public enum TicketType {
    FirstClass(0),
    PremiumClass(1),
    EconomyClass(2);

    private final int elementId;

    Ticket(int elementId) {
        this.elementId = elementId;
    }

    public int getElementId() {
        return elementId;
    }
}

Then you can use it this way:

public void myMethod() {
    ArrayList<Ticket> tickets = new ArrayList<>();
    Ticket ticket = new Ticket();
    ticket.setType(TicketType.FirstClass);
    ticket.setTicketId(1L);
    tickets.add(ticket);

    ticket = new Ticket();
    ticket.setType(TicketType.FirstClass);
    ticket.setTicketId(2L);
    tickets.add(ticket);
}

I don't know why you need an elementId on the ticket type, that's why I just left it there (without using it). Probably you should rename ticketId to just id to keep it simple.

If the ticket's type or ticketId are never changed after assigning them you may want to remove the setters and assign the values in the constructor of Ticket (and make the attributes final).

Even if it's ok that they are changeable, you may introduce such a constructor to have code which is better readable:

In Ticket.java:

public Ticket(Long ticketId, TicketType type) {
    this.ticketId = ticketId;
    this.type = type;
}

Then you can write:

tickets.add(new Ticket(1L, TicketType.FirstClass));

If ticket is a persisted entity (which gets inspected by a framework like Hibernate) you might have to keep a no-args constructor to make it instantiable when loading it from a database.