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

Does the check for unknown positional parameters work? #68

Open
kfischer-okarin opened this issue Aug 23, 2019 · 4 comments
Open

Does the check for unknown positional parameters work? #68

kfischer-okarin opened this issue Aug 23, 2019 · 4 comments

Comments

@kfischer-okarin
Copy link

Hi,

thank you for this useful gem! I have one question about the design choice of how to generate the constructor of the object.

def call
[*required_params, *optional_params, "*", options].compact.join(", ")
end

Why is the '*' included in the parameter list? Wouldn't that just match all positional arguments regardless of whether they are defined or not?

@kfischer-okarin kfischer-okarin changed the title Does the positional parameter check work? Does the check for unknown positional parameters work? Aug 23, 2019
@nepalez
Copy link
Member

nepalez commented Aug 31, 2019

@kfischer-okarin Hi

You are right, the gem does not check the number of params, but just ignores the unknown ones.

This was made by design, because at the moment I got no good solution for adding stars (I mean both * and **) into the DSL. Now the migration to stricter checks is possible in theory, but this decision would cause a huge backward-incompatibility, so I would prefer not to make the change.

@kfischer-okarin
Copy link
Author

I see... I was just asking since it seems to contradict an intuitive reading of the documentation ( https://dry-rb.org/gems/dry-initializer/options-tolerance/ ). So it would maybe be good to add a section about this behaviour....

About the strict parameters..

How about parametrizing the module (builder) and adding parameters strict checking parameters for this?
like... For the Container version this would work out of the box as in

include Dry::Initializer.define(allow_unknown_params: false) -> do
  ...
end

Though I'm not sure what would be an backcompatible way for the normal extend version. Maybe a something like.

extend Dry::Initializer.with_options(allow_unknown_params: false)

@morhekil
Copy link

just bumped in this myself as well. Should the documentation be updated to correctly reflect the fact that positional arguments are not strict? Right now it says the following:

By default the initializer is strict for params (positional arguments), expecting them to be defined explicitly.

which doesn't seem to be case. As the current real behaviour is different from Ruby's own behaviour, I think it's worth documenting this correctly.

nepalez added a commit that referenced this issue Mar 23, 2021
solnic added a commit that referenced this issue Mar 25, 2021
Fix documentation for unknown arguments (#68)
solnic pushed a commit that referenced this issue Mar 25, 2021
@jonspalmer
Copy link

Does the much stricter Ruby behavior around handling args and keyword args make it simpler for us to enforce the same in the gem?

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

No branches or pull requests

4 participants