7

I have a choice.

Option 1:

public class Sample
{
    bool IsRelevant { get; set; }
}

Option 2:

public class Sample
{
}

public class RelevantSample : Sample
{
}

Is there a clear well-known rule how to make this decision?

My research: I have heard about "Replace Conditional with Polymorphism" refactoring before, but it usually deals with large switch statements:

https://sourcemaking.com/refactoring/replace-conditional-with-polymorphism

https://stackoverflow.com/questions/234458/do-polymorphism-or-conditionals-promote-better-design

https://stackoverflow.com/questions/4866873/replace-conditional-with-polymorphism-nice-in-theory-but-not-practical

There is a somewhat related question that describes a different situation (flag as a method argument rather than part of a domain entity): Is it wrong to use a boolean parameter to determine behavior?

7
  • You should use both options with caution, as both will potentially break Likov's substitution principle.
    – David Arno
    Nov 30, 2015 at 23:22
  • 2
    This is a bit off topic, but bool IsRelevant { get; set; } is wrong; it should either be bool IsRelevant { get; protected set; } or, better yet, bool IsRelevant { get; private set; } and be initialized via the constructor.
    – Mike Nakis
    Nov 30, 2015 at 23:47
  • @MikeNakis I agree, just wanted to keep the sample minimal.
    – Den
    Dec 1, 2015 at 17:43
  • 1
    @MikeNakis: mutable data structures are still useful for many situations and are not necessarily "wrong". Jun 29, 2016 at 20:32
  • 2
    @whatsisname I do not deny that mutable classes are useful, but in this particular case the "IsRelevant" property appears to reflect such a fundamental characteristic of the entity, that the property could be eliminated if a subclass was to address this characteristic, as per the OP's example. So, to me it looks like an immutable property.
    – Mike Nakis
    Jun 30, 2016 at 21:59

3 Answers 3

11

I think the language choice is irrelevant. What is important in making the decision is how the information is used.

If Samples behave differently depending on the value of isRelevent then it absolutely makes sense to break it up into three (not two) classes. The base Sample class, a RelevantSample class and a IrrelevantSample class. All sample objects would be instantiated from one of the two later classes (thus following the heuristic that base classes should be abstract.) Other objects can then inform sample objects of events without concerning themselves over whether the sample is relevant or not.

However, if it's more a matter of other objects behaving differently depending on the relevancy of the sample, then you would want to go with option 1 and make isRelevant a field that can be queried.

2
  • Thank you, this sounds like a very clear rule :). My situation is of second variety.
    – Den
    Nov 30, 2015 at 23:27
  • I'd add that you would also go for a single class if isRelevant is a calculated property such as return m_relevance > THRESHOLD ; . Even if it is not the case now, it might be later on. As usual, a design is the result of choices :)
    – user44761
    Dec 15, 2015 at 19:27
13

You have two good answers already; a third reason to stick with the property is that the pattern you are describing is a "one shot" pattern. Everything goes pear shaped when you add a second Boolean. We begin with:

public class Sample
{
    public bool IsRelevant { get; protected set; }
}

and we refactor this into:

public abstract class Sample {}
public class RelevantSample : Sample {}
public class IrrelevantSample : Sample {}

And now we realize, oh, wait, samples can also be frobby or antifrobby:

public abstract class Sample 
{
    public bool IsFrobby { get; protected set; }
}
public class RelevantSample : Sample {}
public class IrrelevantSample : Sample {}

And now how do we move that into the type system?

public class RelevantFrobbySample : RelevantSample {}
public class RelevantAntifrobbySample : RelevantSample {}
public class IrrelevantFrobbySample : IrrelevantSample {}
public class IrrelevantAntifrobbySample : IrrelevantSample {}

And now I want to make a method that takes only frobby samples. How do I do it?

Single inheritance languages require you to choose your "inheritance pivot" extremely carefully because you only get one shot at it.

2
  • Good point and even when C# gets traits I would probably avoid using them for reason given by @Telastyn.
    – Den
    Dec 15, 2015 at 17:43
  • The solution is to use Interfaces instead of Abstract classes. I consider inheritance of implementation a code smell. (My last sentence can spawn a whole conversation which I'm happy to have, but isn't appropriate here. :-)
    – Daniel T.
    Jun 30, 2016 at 13:54
3

Not in C#.

Encoding implied behavior into types is evil in C# and similar languages, because the only way to get info out of the type is if x is T sort of checks (or trickery with dynamic, or reflection). So any change to it (adding a new variant, changing the behavior of a type) mean you get to go into all your consumers, violating the Open Closed Principle.

10
  • 1. I tagged it to get syntax highlighting, info about other languages would be appreciated.
    – Den
    Nov 30, 2015 at 22:50
  • 2. What if I say that change is very unlikely?
    – Den
    Nov 30, 2015 at 22:51
  • @den - in languages with pattern matching, or ingrained dynamic dispatch, it works better, but I don't have enough experience there to say which way to go.
    – Telastyn
    Nov 30, 2015 at 22:53
  • @den - change is inevitable.
    – Telastyn
    Nov 30, 2015 at 22:53
  • Well, it should get better syntax-wise in C#7 with it's pattern matching. I was wondering about mathematical / philosophical /theoretical side of things :).
    – Den
    Nov 30, 2015 at 22:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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