mdeering / attribute_normalizer

Adds the ability to normalize attributes cleanly with code blocks and predefined normalizers
MIT License
475 stars 53 forks source link

Surprising behavior: Calling normalize_attribute before setter is defined results in attribute not being normalized #80

Closed flivni closed 4 years ago

flivni commented 7 years ago

If you call normalize_attribute before a setter is defined, the attribute is not normalized. e.g.

Class Foo < ActiveRecord::Base
  validates :bar, :presence => true
  normalize_attribute :baz
  ...
  def baz=(v)
    ...
  end
end

This is because normalize_attribute looks for method :baz= before it is defined.

It is a natural impulse to call normalize_attribute at the top of the class (near the validators) and then include setters below.

Perhaps there should be a WARNING in the docs? Or perhaps there is a nice way to have the method forwarding late-bound?

dmeremyanin commented 7 years ago

Hi @flivni. You may try to use normalizr instead. It was created just because of this problem.

mdeering commented 4 years ago

I'm closing this. By redefining the original setter method later on I would expect it to override everything in the original setter (unless there was a super call in there or something). If the setter itself did not exist I would also expect to have to call normalize_attribute only after the setter is in fact defined. This matches up with standard ruby and behaviour for method aliasing and chaining.

flivni commented 4 years ago

I humbly disagree. You're right that "standard ruby behavior for method aliasing and chaining" works this way. But I think normalize_attribute is adding new behavior and is therefore more similar to validations or filters which are late-bound. You know how this is all implemented under the covers, but users of your library will not know. And the way it fails is subtle. If you don't like the late-binding approach, perhaps you could WARN or raise if called out of order.

LucasBrandt commented 1 year ago

I agree that a change in the behavior to "just work" when overriding a setter later or a warning when doing that would be helpful. I was overriding a setter on an existing model and I didn't expect this behavior, and only caught a resulting problem because the model had test coverage for the normalization.

Thanks for the library, it's testament to how effective it is that I rarely have to think too much when using it.