Adding support to `use` and `multiple`

[gullitmiranda]

Enhancements

• Adding support to use
• In changesets, use errors instead of raise a exception

  Motivation

• the use makes the objects more flexible.
• support for lists of binary or atoms, and keywords.
• support the multiple

  TODO

• [x] Support a multiple

[avitex]

This is terrific! Would love to see this merged.

[fearenales]

@gullitmiranda One suggestion:

When I tried to convert an invalid element (using both to_atom/1 and to_integer/1), I'm getting the atom :error.

However, if I had it as part of my enum, the atom :error is a valid entry:

enum ~w(new processing processed done error)a

to_integer(:error) #=> 4
to_atom(4)         #=> :error
to_atom(19)        #=> :error (???)

My suggestion is to either return nil or a tuple ({:error, value} for invalid params, {:ok, value} for successful conversions).

Thoughts?

[gjaldon]

@gullitmiranda here are my comments on the proposed changes: Adding support for use - do you have a need for use? I can't think of any use-case where the flexibility of use is needed In changesets, use errors instead of raise a exception - this is great! support for lists of binary or atoms, and keywords. - I like the explicitness keyword lists provide. I'd like to avoid using lists of binary or atoms for defining enums since the only benefit they provide is less typing but at the expense of clarity. support the multiple - what's the use of multiple?

[gullitmiranda]

• support of multiple: The option multiple allows you to use {:array, :integer}, ex: https://github.com/gullitmiranda/ecto_enum/blob/master/test/ecto_enum_test.exs#L71-L92
• example of using use:

defmodule Core.Roles do
  use EctoEnum, multiple: true

  enum ~w(admin default)a

  def in?(role, roles) do
    role = to_integer(role)
    Enum.any?(to_integer(roles), fn(v) -> v == role end)
  end

  def am_i?(conn, %{} = resource) do
    current_user = Guardian.Plug.current_resource(conn)
    current_user && current_user.id == resource.id
  end

  def am_i?(_, _), do: false
end

if you have interest in the features I can do a rebase of the PR. if you do not have interest I can separate the fork in the other package.

[tlvenn]

Hi, I would very much like to see this PR move forward. Here are my comments:

1) use can open some interesting options. For example, we define our enums in multiple locations but we want to keep all of them in the same namespace, let's say under MyApp. Right now we have no other choice but to fully qualify the module name when we define each of our enums.

If use was supported, we could introduce a module_prefix option to specify a module prefix when defining enums using defenum.

2) Changeset errors instead of raising is definitely a good idea.

3) enum vs defenum : enum macro define the enum in place and defenum define the enum inside a module. In our case, we find it convenient to be able to define multiple enums within the same file easily but I definitely see how defining enum in place can be interesting and more flexible. I think we should have both, where defenum is a simple macro defining the module and then calling the enum macro. It would also mean we would be backward compatible.

On the implicit vs explicit ordinal, I think we should support both. Sometimes people dont care about the mapping and using the order is good enough and pretty convenient and in some other cases, being able to define the mapped value is mandatory when you dont want to depend on the order or when you need to map on specific values. So the enum macro must support a keyword list as well.

4) Multiple is pretty interesting and it would be great to be able to specify an encoding/decoding strategy, We could support csv and bitmask for example. And it should be pluggable ideally.

[gullitmiranda]

nice. about the item 4, I think we can add this support in the future.

you need me to do the rebase?

[tlvenn]

I cant really speak for @gjaldon but I am pretty sure it would help a lot if you could enhance the PR to implement 3) so that:

• [ ] defenum is a convenience macro to define an enum inside a module using the enum macro
• [ ] The enum macro supports implicit/explicit ordinals and the tests and documentation should show this

Thanks in advance !

[gullitmiranda]

the defenum support was not removed. you can look in this part of the code: https://github.com/gjaldon/ecto_enum/pull/6/files#diff-0d010fb68ef97ea9bc5287ec440006d3R116

[tlvenn]

Weird when I looked at the code I though it was there but not leveraging the new enum macro.. my bad ;)

[lleger]

Adding support for use - do you have a need for use? I can't think of any use-case where the flexibility of use is needed

I actually ran into this. I wanted to extend the enum module created by defenum to add a module attribute. With the use ability, it would've been trivial. I support this PR 👍

[jnylen]

@gjaldon, Any eta on merging this? I really need the multiple option.

[gen1321]

Yeah i really need it too

[gjaldon]

I added support for use in #77 but haven't done so for multiple. I do not yet see how I could support multiple.

Sorry it took a while on this and I did not merge this PR. I wanted to refactor the internals of EctoEnum to make it easier to support this feature and string-backed enums.