Flipper.register should match any truthy value, not just true

[gareth]

We recently wrote something like the following, hoping to set a generic group for flipper usage.

Flipper.register(:reviewers) do |actor|
  actor.try(:email) =~ /@example.com\z/
end

It didn't work, because String#=~ returns an integer (the position of the match in the string) rather than true/false. Flipper groups only respond if the register block explicitly returns true.

This seems a bit unrubyish, so I wondered if there was a specific reason that only true was allowed rather than (IMO) the more usual convention of any truthy value?

[jnunemaker]

It didn't work, because String#=~ returns an integer (the position of the match in the string) rather than true/false.

Hmm. That is annoying. I can see thinking that would work.

This seems a bit unrubyish, so I wondered if there was a specific reason that only true was allowed rather than (IMO) the more usual convention of any truthy value?

I prefer explicit and safety. It is safer to accidentally or confusingly keep something disabled than to accidentally or confusingly enable a feature. For example, in this case, though confusing that no one was enabled, there was no harm (other than your time spent confused about this and researching). The opposite is not true though. If a truthy-ish value (0, etc.) snuck in and enabled something for a bunch of people without one expecting that to happen, it could be really bad. That said, I can't really think of a good example of where truthy could burn (other than perhaps 0), so maybe I should change this. What are your thoughts? Still feeling truthy would be better?

[gareth]

I still do. Even 0 being truthy is a pretty consistent thing in Ruby, developers (as far as I'm concerned) are likely to understand how if statements work before they get to creating Flipper groups, even if they're previously familiar with languages like Javascript and PHP where 0 is falsey.

I do see your point about accidentally enabling a flipper being worse than the alternative, but I don't think it'll happen - not many methods return zero in a context where it would matter here. The main one I can think of is checking the size of a collection, and even then people are more likely to use a method which returns an actual boolean:

Flipper.register(:noted) do |actor|
  # Enable if user has any notes
  actor.notes.size #=> 0; ok, this would fail if all truthy values were accepted
end

But that's not idiomatic ruby, I think people are more likely to use one of: actor.notes.any? or actor.notes.size > 0.

[gareth]

If you decide not to make a change here, then the documentation for Flipper.register should at least be clear :)

[jnunemaker]

I have a branch locally started to change. Hoping to finish it soon, but have some bigger things on my todo list right now. Feel free to whip something together if you have time.

[jnunemaker]

@gareth opened up #113. Any thoughts?

[gareth]

Aside from the comment I made on the commit, that looks like exactly the right change for us :)