Feature flags best practice: condition inside or outside of a method?

Question

We're using feature flags in order to enable/disable certain features in our system.

I had a discussion with my colleague over what's the standard way of adding feature flags to the code itself:

Consider the following method:

def featured_method
  do_this
  do_that
end

The method is being called from about 15 different places inside our code.

Would you recommend adding the check if the feature is enabled before every call to this method:

if feature_enabled?(:feature_key)
  featured_method
end

Or inside the featured_method itself, like this:

def featured_method
  if feature_enabled?(:feature_key)
    do_this
    do_that
  end
end

The advantage of having the condition inside the method itself is obvious: DRYing up the code, and the fact that when you want to add the feature permanently, you simply remove the condition from within the method.

The advantage of having the condition before every call is that it is very clear whether that method gets executed or not without going into the featured_method code itself, which can save quite a lot of headaches.

I was wondering if there's another solution or a standard for those kind of issues.

There are people who will vote to close _anything_. – Wayne Conrad May 14 '13 at 13:50 — Wayne Conrad, May 14 '13 at 13:50
lol, I couldn't care less mate, no worries :) – Mikey S. May 14 '13 at 14:39 — Mikey S., May 14 '13 at 14:39

score 3 · Accepted Answer · answered May 14 '13 at 10:42

3

I would merge both approaches.

This would lead to DRY code on the callers side. It would not violate the SRP in the feature_method and it would clearly communicate what is going on - if you can find a better name than me:

def may_execute_featured_method
  featured_method if feature_enabled?(:feature_key)
end

def featured_method
  do_this
  do_that
end

The caller would use may_execute_featured_method

answered May 14 '13 at 10:42

Daniel Hilgarth

171,043
40
335
443

5

The downvoter is encouraged to explain the downvote. Otherwise I can't improve the answer. – Daniel Hilgarth May 14 '13 at 10:56
That's actually a nice approach :) – Mikey S. May 14 '13 at 11:24

score 1 · Answer 2 · answered May 14 '13 at 14:53

I would be tempted to split feature keying out into its own module, and use it like this:

class Foo

  include FeatureKeyed

  def foo
    'foo'
  end
  feature_keyed :foo

  def bar
    'bar'
  end
  feature_keyed :bar

end

foo = Foo.new
p foo.foo    # => "foo"
p foo.bar    # => FeatureKeyed::FeatureDisabled

Here's the module:

module FeatureKeyed

  class FeatureDisabled < StandardError ; end

  def self.included(base)
    base.extend ClassMethods
  end

  module ClassMethods

    def feature_keyed(method_name, feature_key = method_name)
      orig_method = instance_method(method_name)
      define_method method_name do |*args|
        raise FeatureDisabled unless feature_enabled?(feature_key)
        orig_method.bind(self).call *args
      end
    end

  end

  def feature_enabled?(feature_key)
    feature_key == :foo
  end

end

Notes:

feature_enabled? hard-codes the eneabled feature names. You would change that.
This code raises an exception if a feature is disabled. The code in your question simply returns. Do what makes sense for your application. If you need different "not enabled" behavior for different methods, then the behavior could be passed to feature_keyed.
method _feature_keyed_ will take a second argument which is the feature key. If missing, the name of the method is used as the feature key.

Very, very elegant, I loved it. – Mikey S. May 16 '13 at 06:59 — Mikey S., May 16 '13 at 06:59

Feature flags best practice: condition inside or outside of a method?

2 Answers2