5

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?

  • Sharing your research helps everyone. Tell us what you've tried and why it didn’t meet your needs. This demonstrates that you’ve taken the time to try to help yourself, it saves us from reiterating obvious answers, and most of all it helps you get a more specific and relevant answer. Also see How to Ask – gnat Nov 30 '15 at 22:48
  • You should use both options with caution, as both will potentially break Likov's substitution principle. – David Arno Nov 30 '15 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 '15 at 23:47
  • 1
    @MikeNakis: mutable data structures are still useful for many situations and are not necessarily "wrong". – whatsisname Jun 29 '16 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 '16 at 21:59
9

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.

  • Thank you, this sounds like a very clear rule :). My situation is of second variety. – Den Nov 30 '15 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 :) – Tibo Dec 15 '15 at 19:27
12

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.

  • Good point and even when C# gets traits I would probably avoid using them for reason given by @Telastyn. – Den Dec 15 '15 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 '16 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.

  • 1. I tagged it to get syntax highlighting, info about other languages would be appreciated. – Den Nov 30 '15 at 22:50
  • 2. What if I say that change is very unlikely? – Den Nov 30 '15 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 '15 at 22:53
  • @den - change is inevitable. – Telastyn Nov 30 '15 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 '15 at 22:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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