Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    primarykey
    data
    text
    <p>Conditional clauses sometimes do result in code which is harder to manage. This not only includes the <code>if</code> statement, but even more commonly the <code>switch</code> statement, which usually includes more branches than a corresponding <code>if</code> would.</p> <h3>There are cases where it's perfectly reasonable to use an <code>if</code></h3> <p>When you are writing utility methods, extensions or specific library functions, it's likely that you won't be able to avoid <code>if</code>s (and you shouldn't). There isn't a better way to code this little function, nor make it more self-documented than it is:</p> <pre class="lang-cs prettyprint-override"><code>// this is a good "if" use-case int Min(int a, int b) { if (a &lt; b) return a; else return b; } // or, if you prefer the ternary operator int Min(int a, int b) { return (a &lt; b) ? a : b; } </code></pre> <h3>Branching over a "type code" is a code smell</h3> <p>On the other hand, if you encounter code which tests for some sort of a type code, or tests if a variable is of a certain type, then this is most likely a good candidate for refactoring, namely <a href="http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism" rel="noreferrer">replacing the conditional with polymorphism</a>.</p> <p>The reason for this is that by allowing your callers to branch on a certain type code, you are creating a possibility to end up with numerous checks scattered all over your code, making extensions and maintenance much more complex. Polymorphism on the other hand allows you to bring this branching decision as closer to the root of your program as possible.</p> <p>Consider:</p> <pre class="lang-cs prettyprint-override"><code>// this is called branching on a "type code", // and screams for refactoring void RunVehicle(Vehicle vehicle) { // how the hell do I even test this? if (vehicle.Type == CAR) Drive(vehicle); else if (vehicle.Type == PLANE) Fly(vehicle); else Sail(vehicle); } </code></pre> <p>By placing common but type-specific (i.e. class-specific) functionality into separate classes and exposing it through a virtual method (or an interface), you allow the internal parts of your program to delegate this decision to someone higher in the call hierarchy (potentially at a single place in code), allowing much easier testing (mocking), extensibility and maintenance:</p> <pre class="lang-cs prettyprint-override"><code>// adding a new vehicle is gonna be a piece of cake interface IVehicle { void Run(); } // your method now doesn't care about which vehicle // it got as a parameter void RunVehicle(IVehicle vehicle) { vehicle.Run(); } </code></pre> <p>And you can now easily test if your <code>RunVehicle</code> method works as it should:</p> <pre class="lang-cs prettyprint-override"><code>// you can now create test (mock) implementations // since you're passing it as an interface var mock = new Mock&lt;IVehicle&gt;(); // run the client method something.RunVehicle(mock.Object); // check if Run() was invoked mock.Verify(m =&gt; m.Run(), Times.Once()); </code></pre> <h3>Patterns which only differ in their <code>if</code> conditions can be reused</h3> <p>Regarding the argument about replacing <code>if</code> with a "predicate" in your question, Haines probably wanted to mention that sometimes similar patterns exist over your code, which differ only in their conditional expressions. Conditional expressions do emerge in conjunction with <code>if</code>s, but the whole idea is to extract a repeating pattern into a separate method, leaving the expression as a parameter. This is what LINQ already does, <em>usually</em> resulting in cleaner code compared to an alternative <code>foreach</code>:</p> <p>Consider these two very similar methods:</p> <pre class="lang-cs prettyprint-override"><code>// average male age public double AverageMaleAge(List&lt;Person&gt; people) { double sum = 0.0; foreach (var person in people) { if (person.Gender == Gender.Male) sum += person.Age; } return sum / people.Count; } // average female age public double AverageFemaleAge(List&lt;Person&gt; people) { double sum = 0.0; foreach (var person in people) { if (person.Gender == Gender.Female) // &lt;-- only the expression sum += person.Age; // is different } return sum / people.Count; } </code></pre> <p>This indicates that you can extract the condition into a predicate, leaving you with a single method for these two cases (and many other future cases):</p> <pre class="lang-cs prettyprint-override"><code>// average age for all people matched by the predicate public double AverageAge(List&lt;Person&gt; people, Predicate&lt;Person&gt; match) { double sum = 0.0; foreach (var person in people) { if (match(person)) // &lt;-- the decision to match sum += person.Age; // is now delegated to callers } return sum / people.Count; } var males = AverageAge(people, p =&gt; p.Gender == Gender.Male); var females = AverageAge(people, p =&gt; p.Gender == Gender.Female); </code></pre> <p>And since LINQ already has a bunch of handy extension methods like this, you actually don't even need to write your own methods:</p> <pre class="lang-cs prettyprint-override"><code>// replace everything we've written above with these two lines var males = list.Where(p =&gt; p.Gender == Gender.Male).Average(p =&gt; p.Age); var females = list.Where(p =&gt; p.Gender == Gender.Female).Average(p =&gt; p.Age); </code></pre> <p>In this last LINQ version the <code>if</code> statement has "disappeared" completely, although:</p> <ol> <li>to be honest the problem wasn't in the <code>if</code> by itself, but in the entire code pattern (simply because it was duplicated), and</li> <li>the <code>if</code> still actually exists, but it's written inside the LINQ <code>Where</code> extension method, which has been tested and closed for modification. Having less of your own code is always a good thing: less things to test, less things to go wrong, and the code is simpler to follow, analyze and maintain.</li> </ol> <h3>Refactor when you feel it's a code smell, but don't over-engineer</h3> <p>Having said all this, you <a href="http://c2.com/cgi/wiki?WhenToStopRefactoring" rel="noreferrer">should not spend sleepless nights</a> over having a couple of conditionals now and there. While these answers can provide some general rules of thumb, the best way to be able to detect constructs which need refactoring is through experience. Over time, some patterns emerge that result in modifying the same clauses over and over again. </p>
    singulars
    1. This table or related slice is empty.
    plurals
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. VO
      singulars
      1. This table or related slice is empty.
    2. VO
      singulars
      1. This table or related slice is empty.
    3. VO
      singulars
      1. This table or related slice is empty.
    1. This table or related slice is empty.
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload