Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setters are not being called in new #381

Open
drujensen opened this issue Feb 16, 2020 · 3 comments
Open

Setters are not being called in new #381

drujensen opened this issue Feb 16, 2020 · 3 comments

Comments

@drujensen
Copy link
Member

drujensen commented Feb 16, 2020

When calling new with a list of params, it should call the setters for a column so you can override the default behavior.

@Blacksmoke16
Copy link
Contributor

Blacksmoke16 commented Feb 16, 2020

Related to #370. This wouldn't be an issue if we stopped creating initializers behind the scenes. Also for whatever its worth, I don't think Granite ever worked like that, as the old cast_to_field method was setting the ivars directly as well.

class User < Granite::Base
  def initialize(params : Hash(String, String?))
    password = params["password"]
    @email = params["email"]
  end
end

@drujensen
Copy link
Member Author

Yeah, I agree. We should treat params as a serializer, similar to json, yaml, or xml. In Amber, we could initialize the object using Class.from_params method.

Question: should serializers call the setter method?

@Blacksmoke16
Copy link
Contributor

Or treat it more like DB::ResultSet and overload #new for HTTP::Params or something like that.

It would be doable to make it configurable. The serializer we use at work (in PHP) allows you to control the access method; whether it should use reflection or public methods. We could do something similar as I can see the pros and cons of each approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants