Reforms validate method semantics

[RSSchermer]

Warning! This is probably a bit nitpicky, but it is itching in the back of my mind so I figured I bring this up, just for consideration. It is very much related to GCorbel/activeform-rails#3 but as this really concerns Reform I figured this would probably be a better place to discuss this.

I recently found out about reform and a really like it a lot, but I have an issue with the #validate method. I have no problem with the method setting both form property values as well as doing validation (no SRP issue imo), I just don't think the current method name properly covers what it does. Out of context I would assume a method called #validate to not alter object state (apart from maybe adding validation errors to a list).

I think I would prefer a method named #bind or #bind_params. I think my ideal flow would be something like this:

...
# Some controller method
@form = ResourceForm.new(Resource.new)

@form.bind(params[:resource])

if @form.valid?
  @form.save
  # Do redirects and stuff
  ...

Such a method could return a boolean indicating passing or failing validation just like the current #validate method (which would basically just make it an alias for the current #validate method) which would allow you to keep it on one line:

...
@form = ResourceForm.new(Resource.new)

if @form.bind(params[:resource])
  @form.save
  ...

I would probably prefer the first example because it is slightly more explicit, but that's really personal preferance.

There might be better names than 'bind', but at least it is a word that does indicate state change to me. I would much rather have a method which I expect to change state return a boolean as a side-effect (basically like activerecord's #save does), than have a method which I expect to just return a boolean change state as a side-effect.

This is just my 2 cents on the matter and I was wondering what your sentiments are on this. In my opinion #validate has a bit of a naming issue and a #bind alias would solve at least my personal problem with it.

[apotonick]

This is an interesting idea. I wanna stress again that #validate does not alter the decorated model itself, only form fields are updated.

I've been thinking about this but currently see no point why you would have bind and then a validate step in a form. The current public API is very limited and that's what it should be, speaking of SRP.

Adding a #valid? method introduces the problem of state - now you as a user have to remember if you already bound values, or not. I wanted to minimize that by exposing as less public methods as possible.

[RSSchermer]

I did understand that #validate only changes the form's state, not the model's. I think the model's state should only be changed through an other explicit method call, #save in this case, and never happen in the same method call as parameter binding. However, even though #validate has no problems in this regard, its behavior is still different from what I expected based on it's name.

I would be perfectly happy with the one-line example, which would mean only adding a #bind alias for validate without any other alterations. I believe this would solve the naming confusion that #validate brings.

As for introducing (making public) a #valid? method, I suppose this is a completely separate point, here's my view on it:

I see a form as an information container with validation rules: it contains a set of values and a set of validation rules for these values. At any point in time this form instance is either valid or invalid. A form that has only been populated with its model's values, without binding any request parameters, is still either valid or invalid. Even an empty model (a new model), could cause the form to be perfectly valid as long as there are no presence constraints. In my mind form validity has not much to do with whether or not new request parameters have been bound. I don't see why the binding of parameters suddenly makes the form gain the ability to be (in)valid whereas before this binding it does not have this ability.

While we're on the topic of naming, I also noticed some confusion on the name of the #save method while reading through some of the issues. I have read suggestions for naming it #sync or #submit. I don't really like any of those names. #save I think has an issue because it is named the same as the activerecord method while it does not necessarily do the same thing. #submit has an issue because there is another moment that could be considered the submitting of the form and that is when the user clicks the button and sends the request, which makes #submit a bit dubious in my opinion. #save and #sync also have an issue in that they have an association with persistence in my mind, but I may want to do something completely different with the form information, which would not necessarily involve persistence. I would like to offer up #process for consideration. It is a much more neutral word in my opinion. The default process happens to be saving the model, but I can override that method to do whatever I please with the form information. It also eliminates the activerecord confusion.

...
# Some controller method
@form = ResourceForm.new(Resource.new)

if @form.bind(params[:resource])
  @form.process
  # Redirects and stuff
  ...

I know I'm being nitpicky about this stuff, I just think naming can have a big impact on the understandability of an API. These are all just ideas and suggestions.

[apotonick]

Hey Roland,

sorry for the late reply. Thanks again for sharing your thoughts. It makes me happy to see people actually think about things and not just accept a shitty API.

From what I understand, you're saying a validation is a state that is query-able at any point in an object's lifecycle. In your understanding, a validation state "lives" in an object.

In my design, a validation is a process that will eventually end in a state. That means, you have to start that process to find out the state. This is why I have Form#validate that starts the validation workflow and returns a state.

Does this make it a bit clearer? What's your thoughts on state vs. process?

[RSSchermer]

I think maybe the terms we're using are a bit confusing. I agree that validation is a process that determines the property of validity of a certain object. I think the issue lies with what object the validity is a property of. In my mind validity is a property of the form and in the current implementation, validity seems to be a property of the parameter hash.

Let's for arguments sake say that the process of validation is not a responsibility of the form, but a responsibility of a separate validator service. I think the current implementation, where validity is a property of the hash, would look like this:

validator = Validator.new(form)

validator.validate(hash) # Is this hash valid, when mapped to the validation rules on form?

I think that the situation in which validity is a property of the form would look like this:

validator = Validator.new()

form.bind(hash) # Map the values of this hash to the properties of the form

validator.validate(form) # Is this form valid, given it's property values and validation rules?

I hope this helps illustrate what I mean with the validity as a property of hash vs property of form thing. I guess to me it makes more sense as a property of the form than as a property of the hash.

[apotonick]

Thanks again for this discussion! I tried to clarify the semantic of validate, sync and save in the README and Reform's 1.0 API is as minimalistic as it could be.

Let me know what you think!!!