Skip to content
This repository has been archived by the owner on Feb 8, 2023. It is now read-only.

Implement the builder pattern to make the Cookies.Attributes immutable. #9

Open
FagnerMartinsBrack opened this issue Sep 2, 2015 · 2 comments
Milestone

Comments

@FagnerMartinsBrack
Copy link
Member

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 FagnerMartinsBrack added this to the v0.1.0 milestone Sep 2, 2015
@JevinMenezes
Copy link
Contributor

JevinMenezes commented Oct 14, 2020

@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 ?

@FagnerMartinsBrack
Copy link
Member Author

FagnerMartinsBrack commented Oct 15, 2020

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);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants