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

Add is: parameter to the length validator #2485

Merged
merged 7 commits into from
Jul 30, 2024

Conversation

Dakad
Copy link
Contributor

@Dakad Dakad commented Jul 27, 2024

This PR is to complement the length validator introduced in #2437.
It adds an exact: param to verify that the given param is exactly equal to a limit.

@Dakad Dakad force-pushed the feature/add-length-exact-validator branch from 81c9762 to 293bd86 Compare July 27, 2024 02:50
@dblock
Copy link
Member

dblock commented Jul 27, 2024

I like it. I think we may want to call this equals or even value to match other value parameters, wdyt?

@Dakad
Copy link
Contributor Author

Dakad commented Jul 28, 2024

I like it. I think we may want to call this equals or even value to match other value parameters, wdyt?

For me, equals is better than value. What about equal without s?
Either way, I'm not too concerned about the naming 😄

@dblock
Copy link
Member

dblock commented Jul 28, 2024

Is equal like min and max? I think value is more like those, but I am flexible :) What does @dhruvCW who introduced this parameter, or @ericproulx think?

Stepping back, I think length should simply accept a value, so

 requires :code, type: String, length: 2

@Dakad
Copy link
Contributor Author

Dakad commented Jul 28, 2024

Is equal like min and max? I think value is more like those, but I am flexible :)

Not sure I understand the 1st question.

If I can add my two cents, I would prefer to split into distinct length validators.
In my company, before the length validator was introduced, we currently have 3 distinct validators for the length, exact_length: , min_length: and max_length: . They have _length suffix because we don't want to confuse the with max or min parameter value and to have use validator to such as max_element:, ....

@dblock
Copy link
Member

dblock commented Jul 29, 2024

I think it's pretty easy to grok length: { min: X, max: Y } and length: 2, would love to hear other (strong) opinions.

Note that we did add length in #2437, and it was released, so switching to 3 validators would be a breaking change, and we should be convinced that it's somehow a lot better if we want to deprecate one and introduce the other.

@ericproulx
Copy link
Contributor

ericproulx commented Jul 29, 2024

Is equal like min and max? I think value is more like those, but I am flexible :) What does @dhruvCW who introduced this parameter, or @ericproulx think?

Stepping back, I think length should simply accept a value, so

 requires :code, type: String, length: 2

Rails's Length Validator uses is for exact length. Ain't shocking for me reading

requires :code, type: String, length: { is: 2 }

Suggestion: having in would be great too to define a range

requires :code, type: String, length: { in: 5..10 }

@dblock
Copy link
Member

dblock commented Jul 29, 2024

I'd be good with is, but Grape also has value elsewhere (not in a validator), so that's why I suggested that.

Suggestion: having in would be great too to define a range

For large ranges doesn't that explode them causing potentially other problems? But otherwise we could support it along with min and max.

I think @Dakad is looking for some consensus. What's your strong opinion @ericproulx?

@ericproulx
Copy link
Contributor

ericproulx commented Jul 29, 2024

I'd be good with is, but Grape also has value elsewhere (not in a validator), so that's why I suggested that.

Suggestion: having in would be great too to define a range

For large ranges doesn't that explode them causing potentially other problems? But otherwise we could support it along with min and max.

I think @Dakad is looking for some consensus. What's your strong opinion @ericproulx?

value is used in except_value_validator and wording makes sense since the name of the validator is except_value. For length, I would stick to length { is: 1 }.

About the range, Ruby will manage its within a Range object. I'm not worried about large ranges since endless ranges are managed since Ruby 2.6.

@Dakad
Copy link
Contributor Author

Dakad commented Jul 30, 2024

Change done!

@Dakad Dakad changed the title Add exact: parameter to the length validator Add is: parameter to the length validator Jul 30, 2024
@dblock
Copy link
Member

dblock commented Jul 30, 2024

To be or not to be, that is the question. I'm good with this, thanks for hanging in here with us @Dakad. Maybe you want to take the range idea on :)

@dblock dblock merged commit 12dc739 into ruby-grape:master Jul 30, 2024
52 checks passed
@Dakad Dakad deleted the feature/add-length-exact-validator branch July 30, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants