shallow_attributes icon indicating copy to clipboard operation
shallow_attributes copied to clipboard

Immutability

Open maxim opened this issue 7 years ago • 4 comments

Great lib, love how small and efficient it is. Would be great if it could produce immutable objects, either frozen by default, or with skipped attribute writers. Curious to hear your thoughts.

maxim avatar Sep 27 '18 17:09 maxim

hey, thanks for feedback! About immutability. I think it's a good idea. I think we can use something like this:

class User
  include ShallowAttributes

  attribute :name, String
  attribute :age, Integer
  attribute :birthday, DateTime
end

user = User.new
user.name = 'Anton' # => okay

class ImmutableUser
  include ImmutableShallowAttributes

  attribute :name, String
  attribute :age, Integer
  attribute :birthday, DateTime
end

user = ImmutableUser.new
user.name = 'Anton' # => raise error

WDYT?

davydovanton avatar Sep 29 '18 19:09 davydovanton

Sounds like a good option. What do you think about per-attribute?

attribute :name, String, writer: false # true/false/:protected

And for any such non-true attribute, it would also be excluded from mass assignment via attributes=.

maxim avatar Sep 29 '18 19:09 maxim

so, I think it can be hard for implementing. Also, usually you need to use full immutable object raise than immutable attribute (only my experience) 🤔

davydovanton avatar Sep 29 '18 19:09 davydovanton

Definitely agree that usually you want full-immutable. I've seen the exception recently where sometimes you have a persistable object and you want to assign id field only after persistence. OTOH, this can also be done by dup-ing it.

Whether it's per-attribute, or full-immutable, your code is really well designed to accommodate it. I was able to get like 80% there by simply overwriting initialize_setter in a subclass with

def initialize_setter(name, *)
  super.tap { private name } 
end

This achieved the effect that you couldn't call a setter from outside, but initialize still worked. However, mutation was still possible via attributes=. I didn't find an easy way to patch that.

maxim avatar Sep 29 '18 20:09 maxim