1

Let's say that I have a Parent class and a Child class, Parent relies on Child to perform, say, some network request:

class Parent {

  ...

  public init(){
    const child = new Child();
    const stuff = child.fetchStuff()
  }
}
class Child {

  public fetchStuff(){
    const stuff = fetch("blah blah");
    return stuff
  }
}

Now, using feature flags, we want to develop a feature for which Child is going to fetch the stuff in a different way, we should also keep the old way of fetching stuff operative.

In order to do that, we are going to use the featureFlags service that for this example we supposed to be magically injected into the classes.

Now on to the question, where the check of the feature flag should exist? Should that be in the Parent or in the Child class?

Solution 1: In the parent class

class Parent {

  private featureFlags;

  public init(){
    const child = new Child();
    let stuff = null;
    if (this.featureFlags.isOn('feature')) stuff = child.fetchStuffB();
    else stuff = child.fetchStuffA();
  }

}

In this example, Child provides two methods to fetch the stuff in the new and the old way.

  • Pros

    • No need for Child to be aware of feature flags, achieving loose coupling
    • Child in this way is easier to test
  • Cons

    • In a way Child knows that there are 2 ways to do a thing, so why should not be aware of the feature flag at all?
    • In the lifecycle of a feature flag, this is removed, whenever we are going to remove it work is going to be required on both the Parent and the Child class

Solution 2: In the child class

Let's suppose that Parent is unchanged and is still calling just fetchStuff:

class Child {

  private featureFlags;

  public fetchStuff(){
    let stuff = null;
    if (this.featureFlags.isOn('feature')) stuff = this.fetchStuffB():
    else stuff = this.fetchStuffA();
    return stuff;
  }

}

  • Pros
    • Parent just want the stuff, it does not care how it is done or how it is fetched
    • When this feature flag is going to be removed, work is required in this class only
  • Cons
    • This class relies on the featureFlags service (I said previously that the featureFlags is magically injected anywhere but the truth is that I don't really like it)
    • Harder to test, in order to test the behaviour of fetchStuff, one need to be aware of the feature flag used internally, breaking in this way encapsulation

I see valid arguments on both sides, what should I go with? I'm more on the side of the first solution because I like that the Child class is transparent about how is doing something and why.

Also this is a question about maintainability, let's say that I have dozens if not hundreds of classes that need to do things differently according to feature flags, should the checks live only in well known places/classes or are we ok in putting this just before the lines of code that determines the change, no matter where they are?

2
  • 4
    One solution you appear not to have considered to to create a Child2 class with the new behaviour, and your DI framework then registers the right class. Commented Jun 23, 2023 at 12:56
  • What if Child is a reasonable big class and only one line of code needs to be done differently? Should I extract that line in another class? Commented Jun 23, 2023 at 13:39

2 Answers 2

5

Where you check the feature flag depends on the scope of the feature.

Checking the feature flag in the Parent class is ideal for cases where you do not want the Child class behavior changed globally. If you only want the new behavior as part of the Parent class, check the feature flag in Parent before deciding which method on Child to call.

If you find that every invocation of Child.fetchStuff() is preceded by checking the feature flag, then solution #2 becomes ideal. The Child class should check the feature flag and execute behavior accordingly. This keeps knowledge of the feature flag limited and easier to clean up.

Another option to consider is creating a sub class of Child, which overrides fetchStuff() with the new behavior.

class Child {
    fetchStuff() {
        // old behavior
    }
}

class NewFeatureChild extends Child {
    override fetchStuff() {
        // new behavior
    }
}

The Parent class would still expect a Child object, which could be of type Child or NewFeatureChild. The code example you provided shows Parent initializing a Child object. Depending on what these classes are responsible for, you might want to initialize Child or NewFeatureChild elsewhere, and pass this in as a constructor parameter for Parent.

Child child;

if (featureFlags.isOn('featureA')) {
    child = new Child();
}
else {
    child = new NewFeatureChild();
}

parent = new Parent(child);

This pushes the check for the feature up the call stack. This might be ideal for situations where Parent is some sort of service class or data access class, which gets configured in a dependency injection container in the composition root of the application. This is not so ideal for domain or business classes, though, which are typically initialized closer to a use case, and further away from the composition root.

6
  • 2
    I think your final option is likely the best long term option. Even if it is a domain class, it would be advantagous (for unit testing if nothing else) to invert its dependency on the class used for fetching data.
    – Paddy
    Commented Jun 23, 2023 at 13:36
  • Definitely a good plan to suggest inverting the dependency here, supports DI frameworks as well. Commented Jun 23, 2023 at 17:15
  • "Another option to consider is creating a sub class of Child" - I see no reason for inheritance here. For better decoupling, rather make a Child interface and implement separate ChildA and ChildB classes.
    – Bergi
    Commented Jun 23, 2023 at 21:18
  • @Bergi, and then what? Copy and paste the entire class? To be honest, inheritance is not my favorite solution here. If used, I would advocate for this being temporary. Once you believe the feature is stable, move the logic to the class it should be in, and delete the derived class. Commented Jun 23, 2023 at 23:11
  • @GregBurghardt Why copy anything? The class only has one method, fetchStuff, making this a classical strategy pattern. Depending on the language (the code might be TypeScript, Java, C#? But probably is meant to be pseudo code), you wouldn't even use a class but just pass a function. If there's multiple methods in the strategies, I'd argue that each strategy would have its own implementation for each method (not sharing code) or the SRP and ISP would be violated.
    – Bergi
    Commented Jun 23, 2023 at 23:24
1

It is difficult to answer with such an abstract example, there are situations where both options are the most ideal. Ultimately, the question is which class "knowing" about the flag makes the most sense. Keeping in mind that the best answer may be—very likely probably is—neither.

Since you mention fetch, let's consider caching as an example:

It might make more sense in the parent if the flag is preferCache is a situation is like: If the flag is set, call child.fetchA(), but if it isn't set, fails, or if the user clicked refresh, call child.fetchB(). Basically any situation where parent needs to know about both methods at run time.

It might make more sense in the child if the flag is cacheEnabled and the situation is: "Is the cache enabled, if so attempt to fetch cached data?" Especially if the cache behavior is constant and neither requires nor accepts input from the parent.

But ultimately, it probably makes sense in neither as caching could be built directly into the mechanism child uses to fetch the data.

In the second or third case, a factory is probably your best bet. Greg's answer (very correctly) points out how to set up multiple versions of child and select which version to pass into parent, but expanding that one step and implementing the factory pattern would move the decision logic out from the instantiation of parent into a separate class and you'd pass the factory into parent instead of the child. (And it's possible that the child should call a factory that provides the necessary change, considering you pointed out that the change is very small and may be out of scope of the parent.) This also avoids repetition if parent is not the only consumer of child and protects you if the flag can be changed at runtime.

This aligns with Philip's comment as well, as most DI frameworks use the factory pattern (implicitly or explicitly) to determine the correct versions of various classes and provide them based on system context.

0

Not the answer you're looking for? Browse other questions tagged or ask your own question.