java-cookie icon indicating copy to clipboard operation
java-cookie copied to clipboard

Implement the builder pattern to make the `Cookies.Attributes` immutable.

Open FagnerMartinsBrack opened this issue 9 years ago • 2 comments

In js-cookie the attributes are specified using an Object Literal instance. In Java we can make a step forward and ensure that the passed attributes are not going to be changed by external code, creating a builder that builds an immutable instance of the Attributes object.

As it currently stands, the invariants of the Attributes object can be changed in a multi-thread environment even after it is passed to the set method and before the return, despite the set method being implicitly locked by the synchronized keyword or not.

FagnerMartinsBrack avatar Sep 02 '15 03:09 FagnerMartinsBrack

@FagnerMartinsBrack Will it be okay by just making the Attributes class as final along with its data members also marked as final and keeping only constructor(s) and getter methods for Attributes class, or something more is expected since you have mentioned a builder pattern way of doing this ?

JevinMenezes avatar Oct 14 '20 19:10 JevinMenezes

The concern here is that the Attributes should not changed twice by different threads running at the same time. Say one thread calls attrs.secure(true) and another one calls attrs.secure(false). Both entrants will modify the variable in an unspecified order. We can try to run tests, read the spec and try to play with synchronized and final keyword in the setter methods. Trying to guess and put final everywhere is not wise as we wouldn't know what we're doing, only guessing.

However, if we make the class immutable, that is, every set returns a new instance instead of this, then we wouldn't have problems of multiple threads modifying the class state as there would be no shared mutable state. Every time the client changes the state of Attributes it would pass the state as a copy to a new Attributes().

Something like this (pseudo-code):

public Attributes {
  private Attributes(newStateOnlyWithPrimitiveValues) {
    this.stateOnlyWithPrimitiveValues = newStateOnlyWithPrimitiveValues; 
  }
  empty() {
    return new StateWithEmptyPrimitiveValues();
  }
  secure() {
    return new Attributes(this.stateOnlyWithPrimitiveValues);
  }
}
private StateWithEmptyPrimitiveValues {}
Attributes.empty().secure(true);

FagnerMartinsBrack avatar Oct 15 '20 00:10 FagnerMartinsBrack