Validating keywords keys

[José Valim]

Hi everyone,

I am working on a new project and yesterday I spent a couple hours on a bug due to a in a keyword list. In a nutshell, I was supposed to pass parenthesis: 10 as keywords to a function but I passed parentheses: 10.

I have fixed the issue by adding the following code:

    for {k, _} <- keyword, k not in [:parentheses, :other_options], do: raise "unknown key #{inspect(k)} in #{inspect(keyword)}"

The code is super straight-forward but I am wondering if we should add it to Elixir to promote said validation. What do you think? Any suggestions on where it should be defined and with which name?

Thank you!

[Andrea Leopardi]

Considering how straightforward the code you showed is, and that for more complex scenarios we have libraries like nimble_options, I might be slightly hesitant to add this to core.

Andrea

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4J8_RG5eeCZSw_c75Q4y19YFt-ipdnTAEa1cE2GnvwjrQ%40mail.gmail.com.

[Kurtis Rainbolt-Greene]

Yes, but think of the valuable hours saved and the amount of code that won't have to be written.

I mean even Valim's own example again has the typo.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAM9Rf%2BJPu8tF2VzNB4beDqO9jc%2BF-SDE6u%3D724EZm9271jY2ug%40mail.gmail.com.

--

Kurtis Rainbolt-Greene,

Software Developer & Founder of Difference Engineers

202-643-2263

[Devon Estes]

Typos are extremely hard to prevent in dynamic data structures since validations need to be implemented at the point of use instead of at the point of creation/definition of the structure. What would stop the developer from writing the typo in their validation, as José did in his example?

It seems to me like if the goal is to prevent typos then a struct would be the way to go.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAMhJPGiKh3uOaY2UNDFYu9x64n-mM7Sqf7iHU09QeAmfOY0mwQ%40mail.gmail.com.

--

_________________
Devon Estes
+49 176 2356 4717
www.devonestes.com

[Wojtek Mach]

I think this would be a great addition to the core.

While there are libraries in this space, as silly as this may seem, solving

this key typo problem seems like solving the 60%-80% case (not to take away

anything from those libraries!)

How about a Keyword.take!/2?

    iex> Keyword.take!([a: 1], [:a, :b])

    [a: 1]

    iex> Keyword.take!([c: 1], [:a, :b])

    ** (ArgumentError) unknown key :c in [c: 1]

  

There are however two problems with it:

1. would people expect that `Keyword.take!([a: 1], [:a, :b])` should fail

   because `:b` is not in the input?

   Maybe the 2nd argument accepts defaults? (I know it probably starts doing

   too much...)

      iex> Keyword.take!([a: 1], [:a, :b])

      [a: 1, b: nil]

      iex> Keyword.take!([a: 1], [:a, b: 2])

      [a: 1, b: 2]

   In fact this could have the following semantics: if there's no default, it's

   a required key:

      iex> Keyword.take!([], [:a, b: 2])

      ** (ArgumentError) missing required key :a

   What's nice is you can later use `Keyword.fetch!/2` that will save you from

   typos.

   But that being said, If the 2nd argument accepts a keyword, then it

   probably shouldn't be called `take!/2` as it no longer matches `take/2`.

2. If you do: `opts = Keyword.take!(..., ...)` and later `opts[:my_key]` you

   still have an opportunity for a typo and you can't necessarily use

   `Keyword.fetch!/2` because optional keys might not be there.

As Devon mentioned, structs are a really cool solution because they provide

rigidity, defaults, and the assertive map access syntax with ".". Creating a

struct for every function that accepts options feels like a bit much though. 

Taking everything above into consideration, perhaps there's:

    iex> Map.something_something!([], [:name, timeout: 5000])

    ** (ArgumentError) missing required key :name

    iex> opts = Map.something_something!([name: Foo], [:name, timeout: 5000])

    iex> opts.timeout

    5000

and I feel like it's still relatively small addition but it's closer to the

"80% solution". No idea how to name this thing though!

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGowJcg_DWYAQsys5f6Ad1nYket8be1Lsrmui8Uh%3DzEAKzWzTQ%40mail.gmail.com.

[Fernando Tapia Rico]

Agreed with Andrea and Devon. And hopefully in the future that typo will be caught by the compiler!? :)

[José Valim]

Wojtek, I originally thought about Map.merge!, where the second argument must be a subset of the first. This way we can check keys and provide default values:

Keyword.merge!([parenthesis: 10], opts)

However, when I tried using this in practice, I realized that default arguments are not always straight-forward to compute. For example, you may want to compute them lazily. You could argue we could set them to nil in said cases, but then we'd mix the absence of a key with nil value, which may not be desired.

Therefore, I concluded that it is probably best to keep those problems separated and validate only the keys. I agree with Andrea that this is small but the benefit I see having it in core is to promote more folks to use it. Both Python and Ruby provide at the syntax-level a convenience that checks only the given keys are expected. So, when it comes to options, both of these languages are allowing us to write assertive code more elegantly than Elixir.

 

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/5E7686D9-1DC6-4830-8C32-7FCAFFE6E706%40wojtekmach.pl.

[Wojtek Mach]

Fair enough, agreed about decoupling the problem. In that case I’d still offer

Keyword.take!/2 that works like this:

   iex> Keyword.take!([a: 1], [:a, :b])

   [a: 1]

   iex> Keyword.take!([c: 1], [:a, :b])

   ** (ArgumentError)

I think take/2 and take!/2 matches struct/2 and struct!/2.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4%2B9YG1YwoK9JGAitzOzikOeo4dXCHyvu%3DjAU6SN1HRocw%40mail.gmail.com.

[José Valim]

The issue is that for take!, I can see two semantics:

1. The map/keyword must have all of the given keys

2. The map/keyword must have at most the given keys

And I think 1) makes more sense intuitively. :(

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/2A76D025-BF9B-45E1-B268-DD23753FEC6C%40wojtekmach.pl.

[Greg Vaughn]

I really like the idea of a convenience function in the Keyword module. One idea that comes to mind:

Keyword.limit_keys!([parentheses: 10], [:parenthesis])
raises or returns 1st param unchanged.

-Greg

> To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4Jx0-0xU76qaury3k5P8WuKjNRj8xUKj1Cz8a0YyuX%2BMA%40mail.gmail.com.

[Bruce Tate]

In my opinion, these are exactly the kinds of functions we should be adding. They address common scenarios that make it easier on beginners, and encourages the practices we want. +1from me. 

-bt

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/F639C82D-9AB5-4CBC-991C-EC895C8037E3%40gmail.com.

--

Regards,

Bruce Tate

CEO

Groxio, LLC.

512.799.9366

br...@grox.io

grox.io

[Michał Muskała]

I presume after validating the options, you’ll have to access them. Why not combine the two? Something like:

 

    [parentheses, other_option] = Keyword.fetch_exact!(opts, [:parentheses, :other_option])

 

Perhaps even supporting defaults:

 

    [other_option, parentheses] = Keyword.fetch_exact!(opts, [:other_option, parentheses: false])

 

The name, of course, has to improve, but I think functionality-wise a function like this would be really useful.

 

Michał.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4Jx0-0xU76qaury3k5P8WuKjNRj8xUKj1Cz8a0YyuX%2BMA%40mail.gmail.com.

[Devon Estes]

I don’t think the issue is with fetching the values, as we already have Keyword.fetch!/2. The root cause of the issue is that there is connascence of name between disparate places about the specific values in a dynamic, unstructured data structure, and that there is no way currently to programmatically check that connascence of name because of the dynamic nature of a keyword list. Adding a new function like this might feel good, and it might even be a helpful selling point to people, but it won’t actually solve the problem.

If we want to have some concept of named arguments to a function like Ruby & Python do, then that‘s a thing I can get behind, but that‘s not what keyword lists are for and would need to be done in a different manner (and would likely require different/new syntax).

Also, if we want to add functions like this because they _look_ like they‘ll be helpful (even if they might not really be), and that this will make the language more appealing to folks, I can get behind that as well as long as folks acknowledge that this won’t actually solve the problem.

But if we want to solve this problem as it was stated (how to give faster feedback about typos) then the only thing I can see that would actually solve the problem and let the developer know they have a typo is to use a struct or to add some additional syntax for passing named arguments to functions that could be checked at compile time and could validate that the needed keys are passed.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/DB7PR07MB3899661710E2B7CE05CD82A0FAD70%40DB7PR07MB3899.eurprd07.prod.outlook.com.

[José Valim]

> But if we want to solve this problem as it was stated (how to give faster feedback about typos) then the only thing I can see that would actually solve the problem and let the developer know they have a typo is to use a struct or to add some additional syntax for passing named arguments to functions that could be checked at compile time and could validate that the needed keys are passed.

I don't think the problem needs to be solved at compile-time. Definitely it would be best but getting feedback at runtime is better than getting no feedback at all.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGowJciiAumHzprUfvpKKPQ_DG9dR44oCkuc_Kam%3DQCaSVWA-Q%40mail.gmail.com.

[Devon Estes]

There are already a ton of ways to validate presence of keys at runtime - Keyword.fetch!/2, Keyword.has_key?/2, Keyword.update!/3, Keyword.replace!/3 - all of which would spot a typo. What would this really add that would provide meaningful feedback that those other functions don’t offer?

I see the bigger issue is that developers need to opt into that feedback by using those functions instead of the others that „fail“ in more quiet ways. Adding more ways to do this type of validation won’t necessarily mean that developers would be any more likely to use them.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JxykeSzY3XCqA31ghQjx6TrqUA8w%3DEQ5mO4KXGLXPM1Q%40mail.gmail.com.

[Zach Daniel]

What would be really awesome is some kind of "rubric" for answering the question "does this belong in the standard library". It could be as simple as something like a set of criteria like "ubiquity", "utility", "new value add", and some scores that it needs to exceed, e.g perhaps greater than a 7/10 for each. I think that we could grade it without worrying about the name, and then have freeform discussion around naming it once we've decided that the function itself belongs in the standard library. This would have two benefits: structuring the conversation around changes to the standard library, and letting people evaluate their idea against that rubric before submitting it. I get that adding extra structure/process around things can be annoying, but the "should we put X in the standard library" conversation is one that comes up enough to potentially warrant it. 

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGowJchNAD8k5YT3HHv7NRv1EJbLop7NPLuUy6fqoKmyPxskgQ%40mail.gmail.com.

[José Valim]

> There are already a ton of ways to validate presence of keys at runtime - Keyword.fetch!/2, Keyword.has_key?/2, Keyword.update!/3, Keyword.replace!/3 - all of which would spot a typo. What would this really add that would provide meaningful feedback that those other functions don’t offer?

This is not the problem that I originally reported though, is it? We don't want to check if a key exists, we want to check there is no other key beyond the given set.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAK-yb0AMW7m38H6OOyT0K%3DL4Usn1Vqv2QBKzkepJ3Hp54Wt%2Bhg%40mail.gmail.com.