Shortcut bang methods are confusing and should be removed from simple_enum

[Startouf]

I've had the same misunderstanding occur with different developers of my team and I believe it might be the case for a lot of people from the Ruby world: the bang shortcut methods generated automatically (like male!) are very confusing since the ! character is often used to indicate "dangerous methods" that will for instance either raise errors or fire DB queries directly.

So when calling

u = User.find_by(...)
u.male! # I've seen many developers think this was enough to save a model

Although in the above example where as_enum behaves like a simple attribute, one would naturally remember to think about whether it is saving or not ; when considering other uses for simple_enum, like modelling some state, this becomes more tricky. For instance, state gems like aasm define bang methods that are actually methods running transitions, and saving is out of the box since some people can define transitions that send emails.

So my point is that I would would advocate against defining shortcut bang methods. I don't see a problem with defining a method user.male or user.gender_male without a trailing bang character.

[drobny]

@Startouf you raise an interesting point, I think from a pure Ruby level this is semantically correct because bang methods usually indicate that the object is going to be modified in some way which in this enum case is true because it writes the value accordingly.

Coming at it from a Rails / associated gems approach first then I guess this concept has been used and abused a bit especially if you are used to the AR convention of bang methods raising exceptions etc.

What do you think of maybe renaming the method to something more suitable i.e. set_to_male for example or (maybe quite extreme) dropping this completely as you have the u.gender = :male method available?

[Startouf]

@drobny I was thinking about just leaving u.gender = :male but the problem is is is not safe. What would happen if you changed the enum list to remove :male and had a bit of code left doing u.gender = :male. It would break more silently since AFAIK when assigning a non-existing enum value it leaves nil. Having a dedicated method name will make it easier to fail fast in this case (for instance with verifying doubles, etc.), which is why I was just in favor of simply u.male without a bang

Also

bang methods usually indicate that the object is going to be modified in some way

For array or similar collection like objects, it makes sense that select!(), reject!() etc. with a bang indicate the array in memory is changed instead of just making another reference to a subset of items, leaving the original array intact (this distinction becomes crucial on batch/intensive operations)

Here, I am not sure what would be the difference of u.male and u.male! if it was not the automatic save (or whatever atomic operation on the DB). Can you think of a scenario where u.male would be used not to change the object in memory ? we have u.male? to check for the enum state already, I'm not sure what use there would be to distinguish bang and non-bang methods.

[lwe]

Interesting point yes, I'd be OK with removing these methods (without a replacement), people can always use:

u.gender = :other
u.save

# or
u.update_attribute :gender, :other

However, the implications would be to raise the major version, as this is backwards incompatible. If we go that way, can we do the following at first:

• Deprecated the bang and even object.xyz methods