gjaldon / ecto_enum

Ecto extension to support enums in models
MIT License
563 stars 131 forks source link

Project status #40

Closed yordis closed 6 years ago

yordis commented 7 years ago

Sorry I create an issue for this but I don't see any recent activity with a bunch of good PRs done.

Is this project maintained or not? If not, would you add somebody as contributor or pass membership?

ryanwinchester commented 7 years ago

Anybody???

yordis commented 7 years ago

@ryanwinchester and anyone I would recommend to move to https://github.com/KamilLelonek/exnumerator

At the end of the day, there is only one reason to use this package (when you want to use enum at the database layer), that is a good and bad thing depending of many factors.

But I would argue you that for 98% of time you don't need this and it's just fine to use something like exnumator IMO

ryanwinchester commented 7 years ago

Thanks @yordis I'm just using my fork for now :)

leifg commented 7 years ago

I reached out to the maintainer (email in in git log) about a month ago and did not get an answer...

I don't have high hopes for this project to continue.

yordis commented 7 years ago

😥 yeah

ghost commented 7 years ago

It's not that hard to use postgres enum types without this or any other library, actually. Here's an example.

TL;DR, here's all you need to write (for an imaginary invoice_status enum)

defmodule Invoice.Status do
  @moduledoc """
  The status of payment; whether the invoice has been paid or not.

  Enumeration members:
  - `:payment_automatically_applied`
  - `:payment_complete`
  - `:payment_declined`
  - `:payment_due`
  - `:payment_past_due`
  """

  @behaviour Ecto.Type

  statuses = [
    :payment_automatically_applied,
    :payment_complete,
    :payment_declined,
    :payment_due,
    :payment_past_due
  ] |> Enum.map(fn status ->
    {status, to_string(status)}
  end)

  @doc "Returns the underlying schema type for the custom type"
  @spec type :: :invoice_status
  def type, do: :invoice_status

  @doc "Casts the given input to the custom type"
  @spec cast(atom | binary) :: {:ok, atom} | :error
  def cast(status)

  for {atom, string} <- statuses do
    def cast(unquote(atom)), do: {:ok, unquote(atom)}
    def cast(unquote(string)), do: {:ok, unquote(atom)}
  end

  def cast(_other), do: :error

  @doc "Loads the given term into a custom type"
  @spec load(binary) :: {:ok, atom}
  def load(status)

  for {atom, string} <- statuses do
    def load(unquote(string)), do: {:ok, unquote(atom)}
  end

  @doc "Dumps the given term into an Ecto native type"
  @spec dump(atom | binary) :: {:ok, binary} | :error
  def dump(status)

  for {atom, string} <- statuses do
    def dump(unquote(atom)), do: {:ok, unquote(string)}
    def dump(unquote(string)), do: {:ok, unquote(string)}
  end

  def dump(_other), do: :error
end
gjaldon commented 6 years ago

@yordis @ryanwinchester @leifg I've just published latest Ecto 1.1 which only loosens dep requirements which was one of the main issues. Was on sabbatical from software development and back now.

@yordis what good PRs are you referring to btw?

gjaldon commented 6 years ago

@idi-ot the point of this library is that you no longer have to manually set that up for your project. It does some metaprogramming underneath to generate similar code as yours.

ghost commented 6 years ago

@gjaldon I know (actually, I think the code I posted here is a bit more clean than that in the library, would you accept a PR?). I just want people to know that there is a simple way to use eum types with ecto without any additional libraries.

gjaldon commented 6 years ago

@idi-ot the library does metaprogramming so it won't be as clean as code without metaprogramming. Another advantage of this lib is that you may define multiple custom Enums in just a few lines of code. The library is small and focused and does what it needs to do without much complexity.

ghost commented 6 years ago

I meant that it's both cleaner and more efficient to define a lookup table out of multiple function heads, than to use several literal maps, which is the current approach.

Anyway, if you believe that the library is good as is, I won't insist.

gjaldon commented 6 years ago

@idi-ot ahh did not realize that's what you meant! Don't know about the efficiency. Haven't really looked into refactoring it. It does seem cleaner. PR is welcome and would be happy to merge if it ends up cleaner 👍

gjaldon commented 6 years ago

Feel free to reopen if this needs to be revisited

ghost commented 6 years ago

@gjaldon on efficiency: https://github.com/idi-ot/ecto_enum_bench

Finished in 49.56 seconds

## EnumBench
benchmark name  iterations   average time
load custom       10000000   0.20 µs/op
load ecto_enum    10000000   0.25 µs/op
dump custom       10000000   0.31 µs/op
cast custom       10000000   0.32 µs/op
cast ecto_enum    10000000   0.60 µs/op
dump ecto_enum    10000000   0.79 µs/op
## IntBench
benchmark name  iterations   average time
load custom      100000000   0.04 µs/op
load ecto_enum    10000000   0.11 µs/op
dump custom       10000000   0.34 µs/op
cast custom       10000000   0.34 µs/op
cast ecto_enum    10000000   0.81 µs/op

In absolute terms, the difference is almost non-existent, but in relative -- lookup table out of functions is twice as fast as literal map. I've tried to use this method in ecto_enum, but can't get the code to compile ...

ghost commented 6 years ago

dump ecto_enum didn't finish since it raised (should've returned :error instead according to ecto type spec):

** (exit) an exception was raised:
    ** (Ecto.ChangeError) `"akjsdfgh"` is not a valid enum value for `EctoEnumIntStatus`. Valid enum values are `[1, 2, 3, 4, :registered, :active, :inactive, :archived, "active", "archived", "inactive", "registered"]`
        lib/ecto_enum_int_status.ex:2: EctoEnumIntStatus.dump/1
        bench/int_bench.exs:97: IntBench."bench: dump ecto_enum"/3
        (stdlib) timer.erl:197: :timer.tc/3
        lib/benchfella.ex:313: anonymous fn/7 in Benchfella.measure_n/6