Enum.keys returns strings

[matthewrudy]

From your example

class User < ActiveRecord::Base
  as_enum :gender, female: 1, male: 0
end

# expected
user.gender == :female
user.genders.key(1) == :female

# but
user.genders.keys == ["male", "female"]

ie. the enumerated values always appear as symbols everywhere except when calling keys.

[lwe]

Yes, because the internal representation is all strings, can you give an example where this difference is an issue?

[matthewrudy]

Our code was relying upon User.genders.keys.include?(user.gender)

Something like this

people = [alice, bob, charlie]

User.genders.keys.each do |gender|
  these = people.select{|p| p.gender == gender}
  do_something with: these
end

So to make it work, we'd have to explicitly cast

[lwe]

You are totally right that SimpleEnum::Enum is inconsistent with the return values given and in fact there's only one place where to_sym is used:

simple_enum/lib/simple_enum/enum.rb

Line 18 in 2189bbd

key.to_sym if key

- which from an API perspective "feels" wrong too.

The current flow works as follows:

1. hash as input given via as_enum, e.g. { female: 1, male: 0 }
2. is passed first to the Hasher to normalize the hash

   simple_enum/lib/simple_enum/hasher.rb

   Line 20 in 2189bbd

   def self.map(values, options = {})

   - this basically converts Symbols to Strings and also helps to support items to be stored as strings etc. This method still returns a hash.
3. resulting hash is passed to SE::Enum and used in there

PS: It works similar to HashWithIndifferent access in this regard, by e.g. working with any kind of keys, but internally they are represented as strings

Looking back at the code, I really think this should have been designed differently, as the to_sym in SE::Enum feels a bit like a hack to make the comparisons and result values work, also it's weird when the input is a string, it returns magically a Symbol, e.g.

class Hello
  as_enum :foo, %w{bar baz}
end
hello = Hello.new(foo: "bar")
hello.foo # => :baz # Why?!

Therefore I can see two possible solutions, but not sure which one is "correct"

1. Change API to be consistent and ensure to return appropriate type that was used as input hash -> this will probably break existing code
2. Change assumption and define that you can throw anything at simple_enum and it always operates under the assumption that Symbols are to be used and returned, because it looks prettier / more ruby-like -> i.e. change SE::Enum#keys to return symbols as well...

[matthewrudy]

With 1.6 it seems the input type remained intact (strings were strings, symbols were symbols)
So some of my problems today came from these changes in 2.x.

I guess that 2.x deliberately started returning symbols for everything.

If it were me, I'd probably just make sure that things were consistent,
and consider what changes a 3.x release might allow.

[lwe]

Sadly, making it consistent, will break the existing API.

What I did push for in 2.x was that Symbols is how it should be used, this is also why it returns a Symbol (i.e. that to_sym cast). With the introduction of %i{foo bar} the other option could be to ensure to return whatever was used (e.g. retain 1.x behaviour), assignment would still work with both, strings and symbols, but returned in case of e.g. obj.gender would be the type defined:

class Syms
  as_enum :foo, %i{bar baz}
end
a.foo # => :bar
a.foo = "baz"
a.foo # => :baz

class Strs
  as_enum :foo, %w{bar baz}
end
b.foo # => "bar"
b.foo = :baz
b.foo # => "baz"

Not sure what is better, honestly... any opinions / voices?

[lucke84]

I see no reason to support both strings and symbols: it's confusing and inconsistent (you don't know what to expect, makes the dev think too much). I think your idea of dropping strings support in favour of symbols is the right one.

As @matthewrudy suggested, it might be time for a 3.x :)

[lwe]

Cool, this would have been my preferred solution too :) anyone willing to make a PR for this?

Update: and yep, there's going to be a 3.x release due to semver 👍

[lucke84]

So @lwe are we agreeing that we should not allow defining enum values as strings anymore?

class Hello
  as_enum :foo_1, %w{bar baz} # not good
  as_enum :foo_2, %i{bar baz} # good
end

[lwe]

No, I think we agree that the output should always be symbols and it should accept anything.

[lwe]

To clarify 😄 I think interfaces should accept anything being thrown at them

class Hello
  as_enum :foo, %w{bar baz} # all right, we can do that
  as_enum :boo, "bar" => 1, :baz => 23 # well, if you like that!?
  as_enum :zoo, %i{bar baz} # atta boy
end

similar to how we accept strings, symbols and indicies as setters: hello.zoo = "baz" - but we should be strict in what we return / build. So I propose that SE::Enum should only ever expose Symbols as keys to the end user. This means that for all instances of foo, boo and zoo above the keys method should return [:bar, :baz].

[lwe]

Please move discussion to the PR, thus I'm closing this ticket.