24

According to anti-if campaign it is a best practice not to use ifs in our code. Can anyone tell me if it possible to get rid of the if in this piece of code ? (switch is also not an option, The point is to remove the conditional logic, not replace ifs with similar language constructs)

if(s == "foo")
{
    Writeln("some logic here");
}
else if(s == "bar")
{
    Writeln("something else here");
}
else if(s == "raboof")
{
    Writeln("of course I need more than just Writeln");
}

(language: Java or C#)

JasonMArcher
  • 14,195
  • 22
  • 56
  • 52
Omu
  • 69,856
  • 92
  • 277
  • 407
  • 4
    I think we need more context. What's `s` and where does it come from? – R. Martinho Fernandes Jan 04 '10 at 12:34
  • 88
    What lunatic is running an 'anti-if' campaign, and on what basis? – bmargulies Jan 04 '10 at 12:36
  • s is a string, it comes as a parameter in the function where this code is – Omu Jan 04 '10 at 12:36
  • 1
    http://www.antiifcampaign.com/ – Omu Jan 04 '10 at 12:37
  • 6
    for this use case i would stay with an if or a switch statement. the anti-if campaign is targeting IFs that influence objects state. these should can be circumnavigated by using polymorphism (e.g. multiple implementations of the same base class to model these different states). – fasseg Jan 04 '10 at 12:40
  • 2
    I agree with some of the ideas behind the anti-if campaign, but I think removing this particular case is going too far. – R. Martinho Fernandes Jan 04 '10 at 12:41
  • 13
    the antiifcampaign is about eliminating IFs based on *type* (`if (type == BTP) { ... } else if (type == BOT) { ... }`), replacing this by `type.method()`. the type in your example is the same (String), so "anti-if" just doesn't apply. you much time did you spend on antiifcampaign.com? – ax. Jan 04 '10 at 12:45
  • 1
    Switch is not option. The point is to remove the conditional logic, not replace ifs with similar language constructs – Omu Jan 04 '10 at 12:46
  • 12
    AntiIfCampaign: "LESS IFs, MORE POWER" tsk, tsk. Don't they know it should be 'FEWER' not 'LESS' :-) – Gordon Mackie JoanMiro Jan 04 '10 at 12:55
  • By the way, it seems that the campaign responds to your question very clear in this example: http://www.antiifcampaign.com/articles/the-simplest-anti-if-code.html – serhio Jan 04 '10 at 13:06
  • no, this if for types, I have a string ;) – Omu Jan 04 '10 at 13:23
  • 4
    Some people are spending too much time in an ivory Chateau d'If. – bmargulies Jan 04 '10 at 13:35
  • 4
    "The point is to remove the conditional logic" lol - you mean, the point is to hide the conditional logic within the classes. You still have to deal with either creating a new method for each type or collection of objects being passed into the function, or deal with polymorphism, or both. The processor is still processing conditional branches. So all the website is saying, is, "If you're programming in an OO language, use the OO features!" – Adam Davis Jan 04 '10 at 17:09
  • @smeg4brains- I agree. Hence my answer. – RichardOD Jan 05 '10 at 10:07

18 Answers18

76

Here's one way... :)

delegate void DoStuff();

...

IDictionary<string, DoStuff> dict = new Dictionary<string, DoStuff>();
dict["foo"] = delegate { Console.WriteLine("some logic here"); };
dict["bar"] = delegate { Console.WriteLine("something else here"); };
dict["raboof"] = delegate { Console.WriteLine("of course I need more than just Writeln"); };
dict["foo"]();
cletus
  • 616,129
  • 168
  • 910
  • 942
  • 2
    I would have accepted this as the correct answer for C#, too bad I can't accept 2 answers :) – Omu Jan 04 '10 at 15:15
  • 5
    You can use the `Action` type instead of declaring a delegate. – Robert Rossney Jan 06 '10 at 01:20
  • 1
    @RobertRossney - can you give an example of using Action type? – Mark Karwowski Feb 24 '14 at 14:15
  • 1
    @MarkKarwowski This [MSDN reference](http://msdn.microsoft.com/en-us/library/018hxwa8%28v=vs.110%29.aspx) and [this SO answer](http://stackoverflow.com/a/23468276/3155705) provide some example usage scenarios. – JW Lim May 06 '14 at 02:21
20

Make use of the strategy pattern.

In Java terms:

public interface Strategy {
    void execute();
}

public class SomeStrategy implements Strategy {
    public void execute() {
        System.out.println("Some logic.");
    }
}

which you use as follows:

Map<String, Strategy> strategies = new HashMap<String, Strategy>();
strategies.put("strategyName1", new SomeStrategy1());
strategies.put("strategyName2", new SomeStrategy2());
strategies.put("strategyName3", new SomeStrategy3());

// ...

strategies.get(s).execute();
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 12
    This *really* isn't what the Strategy pattern is for. – Draemon Jan 04 '10 at 12:49
  • 1
    Map should support `map.getOrElse(s, default)`. This is just plain ugly code: `(strategies.contains(s) ? strategies.get(s) : default).execute();` – Thomas Jung Jan 04 '10 at 12:49
  • @Thomas: And Strategy s = strategies.get(s); if(s!=null) s.execute(); is bad why? It's the ternary operator that makes that line ugly. – Draemon Jan 04 '10 at 12:52
  • @Draemon: Having to handle null and not being able to define a default value is ugly. BalusC's workaround does work (only) in this context. – Thomas Jung Jan 04 '10 at 12:55
  • actually if you do strategies.put(null, new DoNothingStrategy()); and ask for strategies.get("keyDoesntExist") you will still going to get null ;). So I guess it's better to check for null – Omu Jan 04 '10 at 15:13
  • but checking for null requires *horrors* an *if statement*. – bmargulies Jan 04 '10 at 17:00
  • @BalusC: The strategy pattern is for passing some sort of algorithm/policy/strategy into a system, which will generally use that one strategy for the objects it's handling. You're *decoupling* those objects from the operations upon them. What you want here is a very close mapping between the key string and the operation, so you would be better wrapping that key string and operation in a class of their own. – Draemon Jan 04 '10 at 17:16
  • @Thomas: how is strategies.contains(s) different from "having to handle null"? If you want a default value, just do if(s==null) s = default;. Or you could do Strategy s = default; if(strategies.contains(s)) s = strategies.get(s); s.execute(); Perfectly clean if you ask me. – Draemon Jan 04 '10 at 17:19
  • 7
    These comments are just making me think IFs are a very good idea :) – DanSingerman Jan 04 '10 at 17:57
19

Make an associative data structure. Map<String, String> in Java, IDictionary<string, string> in C#. Initialize it at the beginning of time, and then ...

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
bmargulies
  • 97,814
  • 39
  • 186
  • 310
12

write classes with virtual methods which is derived from your abstract base class SomeThingWriter.

then every class which are derived from base class should implement a function like writeSomething or whatever you want.

abstract class MyBaseClass
{
     public abstract void writeSomething();
}

class DerivedClass1 : MyBaseClass
{
    public override void writeSomething()
    {
        Writeln("something else here  1");
    }
}

class DerivedClass2 : MyBaseClass
{
    public override void writeSomething()
    {
        Writeln("something else here  2");
    }
}

than just call like

MyBaseClass c = new DeriveClass1();
c.writeSomething();
c = new DerivedClass2();
c.writeSomething();
main--
  • 3,873
  • 1
  • 16
  • 37
ufukgun
  • 6,889
  • 8
  • 33
  • 55
12

Looking at the campaign, it's very poorly explained. There's nothing wrong with ifs, but in certain cases they can indicate that you're not using OOP to its full potential.

What the campaign is trying to promote is increased use of polymorphism in order to decouple calling code from the type of object it is looking at.

You would use some smarter object instead of s being a string:

interface I {
  public String getName();
  public void doSomething();
}

class A implements I {
  public String getName() { return "one"; }
  public void doSomething() { ...; }
}

class B implements I {
  public String getName() { return "two"; }
  public void doSomething() { ...; }
}

Then you can replace the ifs with:

I obj = ...get an A or B from somewhere...;
obj.doSomething();
Draemon
  • 33,955
  • 16
  • 77
  • 104
7

In some cases it might be legit to avoid the if structure

in others its just plain idiocy to try to avoid if.

While the examples provided to avoid the if structure are valid alternatives you should ask yourself this:

Why am i making my code unnecessarly complicated to avoid a simple if structure ? If the only reason is that you have to because of the anti-if campaign then its bad reason

Peter
  • 5,728
  • 20
  • 23
7

Java

Use an enum which implements a certain method.

enum MyEnum{

    foo{
        public void mymethod(String param1, String param2){
            //dostuff...
        }
    },

    bar{
        public void mymethod(String param1, String param2){
            //dostuff...
        }
    };

    public abstract void mymethod(String param1, String param2);
}

Then in your class :

MyEnum.valueOf(mystring).mymethod(param1, param2);
glmxndr
  • 45,516
  • 29
  • 93
  • 118
5

First of all, be very attentive when reading such "anti" campaigns.

  • Ask yourself if Anti IF campaign would like eliminate the logic in the applications?!
  • The ideas could have a good application in one situation and a stupid in another one. Be reasonable.
  • It may be possible that multiple usage of IF may encumber the reader of the code. but this is any reason to eliminate the if from your code, more that than, this is almost impossible.
  • By the way anywhere in the MS design guidelines is indicated do not use if (like is done, by e.g. for the goto statement usage of which is not recommended)...

C#

    switch (myStringVar)
    {
        case "one": doSomething();  break;
        case "two": doSomething(); break;
        case "three": doSomething(); break;
        default: doSomething(); break;
    }

Finally, it reduces this code to the if s... so, only for readability is better, not for performance.

Actually, if Microsoft believes that switch (in c#) is better to replace with if's - OK, I will use (in the concrete situation that you described) the switch.

By the way, it seems that the campaign responds to your question very clear in this example

DhruvJoshi
  • 17,041
  • 6
  • 41
  • 60
serhio
  • 28,010
  • 62
  • 221
  • 374
  • 5
    The point is to remove the *conditional* logic, not replace ifs with similar language constructs. – R. Martinho Fernandes Jan 04 '10 at 12:37
  • @Martinho: I got the point. Finally if Microsoft believes that IF could be replaced by something else, they could replace the switch implementation. This code, however, facilitates the readability, so I don't see any worry about do not using the conditional logic in the given example. – serhio Jan 04 '10 at 12:42
  • @serhio: I'm not sure I understand what you mean, but the Anti-If campaign is not associated with Microsoft. – R. Martinho Fernandes Jan 04 '10 at 12:53
  • @Martinho: I proposed in the described situation to use **switch**. Now, for C#, Microsoft implemented it using if logic. So, I think there is nothing bad using if. By the way anywhere in the MS design guidelines is indicated do not use **if** (like is done, by eg. for **goto** statement that is not recommended)... – serhio Jan 04 '10 at 13:01
4

i'd like to point out that so far, every answer to this question with a code example has a solution that is far more complicated than the original code, and likely much slower.

this is a classic case of an optimization being performed in entirely the wrong context. in some cases, code will become clearer through using OO properly, such as eliminating long chains of type checks. however, simply removing all if statements for the sake of removing them only serves to obfuscate your code.

the if statements (conditional jumps) are still going to happen, be it in your code or the interpreter. keeping them lexically close has many readability and maintenance advantages that are lost through excessive OO use. there is a balance that must be struck between local vs distant logic, but it should never spill over into obfuscation.

for the question at hand, the clearest construct that will avoid the if is probably a hash table / associative array containing anonymous functions, which, for a small number of keys, is effectively just a slow switch statement.

Eric Strom
  • 39,821
  • 2
  • 80
  • 152
4

The example you have given I would not change (though I guess you realise it wouldn't need changing)- I'm guessing you are using it as a representational example.

In Fowler's Refactoring book, he discusses the Replace Conditional with Polymorphism. That's what I see as a good use to replace if/switch statements (where appropriate).

RichardOD
  • 28,883
  • 9
  • 61
  • 81
3

I don't think you are making a fair comparison here.

From the look of it the Anti-if campaign is just about practicing a better design approach.

However in your case you can see from all the above examples that if can not be removed from the surface and will always exist somewhere down in the center.

And why exactly is that?

Well If is a general purpose of life. I don't mean to say start coding if every where but in general without if there is no differentiation, if brings decisions and purpose, if that wasn't there then every object in the world would just execute as its suppose to without even knowing anything other then it self. And very simple you wouldn't have asked this question. :)

Syed M Shaaf
  • 194
  • 1
  • 3
3

I think you are looking for Factory Patterns.

Pankaj
  • 4,419
  • 16
  • 50
  • 72
1

You can conceivably do something similar to the "strategy" pattern above using a map of Method calls instead:

public class FooOrBar {
private Map<String, Method> methodMap = new HashMap<String, Method>();

public FooOrBar() {
    try {
        methodMap.put("foo", this.getClass().getMethod("doFoo", new Class[0]));
        methodMap.put("bar", this.getClass().getMethod("doBar", new Class[0]));
    } catch (NoSuchMethodException n) {
        throw new RuntimeException(n);
    }
}

public void doSomething(String str) {
    Method m = methodMap.get(str);
    try {
        m.invoke(this, null);
    } catch (Exception n) {
        throw new RuntimeException(n);
    }

}

public void doFoo() {
    System.out.println("foo");
}

public void doBar() {
    System.out.println("bar");
}

public static void main(String[] args) {
    FooOrBar fb = new FooOrBar();
    fb.doSomething("foo");

}

}

Matthew Flynn
  • 2,210
  • 19
  • 27
1

Abuse the ternary operator, at least in C#:

Action result = 
            s == "bar" ? (Action)(() => { Console.WriteLine("bar"); }): 
            s == "foo" ? (Action)(() => { Console.WriteLine("foo"); }) :
                         (Action)(() => { Console.WriteLine(); });

Actually, I take that back... never EVER do this. Use a switch.

mletterle
  • 3,968
  • 1
  • 24
  • 24
  • For one condition it would be not thaaat bad Action result = s == "bar" ? (Action)(() => { Console.WriteLine("bar"); }): (Action)(() => { Console.WriteLine("foo"); }) – BigChief Nov 10 '14 at 19:56
1

I read http://www.antiifcampaign.com/articles/the-simplest-anti-if-code.html and I think that the medicine is worse than the disease. Much, much worse. You required to invest up front in some heavy OO machinery to solve a possible (improbable?) future problem.

David Soroko
  • 8,521
  • 2
  • 39
  • 51
1

A little late to the party, but combining the C# dictionary answers from MRFerocius and cletus gives the following implementation of bmargulies's answer:

private Dictionary<string,Action> data = new Dictionary<string, Action> {
    {"foo", () => Console.WriteLine("Some logic here")},
    {"bar", () => Console.WriteLine("something else here")},
    {"raboof", () => Console.WriteLine("of course I need more than just WriteLine")},
}

public static void main(String[] args) {
    data["foo"]();
}
  • If the key doesn't exist in the dictionary, using it in the indexer will throw an exception.
  • Multiple actions can be composed:

    • There can be multiple calls to different methods, using multiline lambda syntax:

      {"foobar", () => { data["foo"](); data["bar"](); }

    • As Action is a delegate type, multiple methods can be attached to a single delegate instance and that delegate instance set as the value; they will be called sequentially when the delegate is invoked:

      public static void main(String[] args) { data["foobar"] = data["foo"] + data["bar"]; //This will invoke first data["foo"] then data["bar"] data["foobar"](); }

      For methods not referenced via the dictionary, this can also be done in the collection initializer:

      {"foobar", (Action)method1 + method2}

Community
  • 1
  • 1
Zev Spitz
  • 13,950
  • 6
  • 64
  • 136
0

Here goes mine. Using LINQ and Factory Pattern :D

class FactoryString
    {
    static FactoryString()
    {
    private static Dictionary<string, string> dictionary = new Dictionary<string, string> 
    { 
        {"foo", "some logic here"},
        {"bar", "something else here"},
        {"raboof", "of course I need more than just Writeln"},
    };
}

public static string getString(string s)
{
    return dictionary.Single(x => x.Key.Equals(s)).Value;
}

}

static void main()
{
  Console.WriteLine(FactoryString.getString("foo"));
}
MRFerocius
  • 5,509
  • 7
  • 39
  • 47
  • The only LINQ in here is using `Single`; you can get the same effect (single result per key, exception if not found) by using the indexer of the dictionary. – Zev Spitz Oct 07 '14 at 06:50
0

My general perspective on this kind of problem is not that if statements are bad, it's that it's easier to debug data than it is to debug code.

Here's a non-trivial example from production code. This may look a little complicated at first blush, but at its core it's really simple: depending on the disposition code on a charge row, we need to perform an update to some of its related sentence rows. But we pick different sentence rows, and perform different kinds of updates on them, for different disposition codes.

This is a relatively simple example - there are only five disposition codes, two tests, and two types of updates. Even so, this is vastly simpler than what it replaced. Also, it's a lot easier to tell just from looking at the code that it does what the requirements say it should do, since the mappings in the code correspond to tables in the requirements document. (Before I wrote this code, I had to rewrite the requirements document so that this stuff was all defined in a table. The original code was a mess because the requirements were a mess too. Rewriting the requirements to make them clearer exposed bugs in the requirements, too.)

It's worth emphasizing that it's pretty easy to write a unit test that covers 100% of this code. It's also worth emphasizing that the complexity of this code scales linearly with the number of disposition codes, predicates, and updates that it supports; if case or if statements were used, it would scale exponentially.

    /// <summary>
    /// Update a sentence's status to Completed [401110]
    /// </summary>
    /// <param name="senRow"></param>
    /// <param name="eventDate"></param>
    private static void CompleteSentence(DataRow senRow, DateTime eventDate)
    {
        senRow.SetField("SenStatus", "401110");
        senRow.SetField("SenStatusDate", eventDate);
    }

    /// <summary>
    /// Update a sentence's status to Terminated [401120]
    /// </summary>
    /// <param name="senRow"></param>
    /// <param name="eventDate"></param>
    private static void TerminateSentence(DataRow senRow, DateTime eventDate)
    {
        senRow.SetField("SenStatus", "401120");
        senRow.SetField("SenStatusDate", eventDate);
    }

    /// <summary>
    /// Returns true if a sentence is a DEJ sentence.
    /// </summary>
    /// <param name="senRow"></param>
    /// <returns></returns>
    private static bool DEJSentence(DataRow senRow)
    {
        return Api.ParseCode(senRow.Field<string>("SenType")) == "431320";
    }


    /// <summary>
    /// Returns true if a sentence is a Diversion sentence.
    /// </summary>
    /// <param name="senRow"></param>
    /// <returns></returns>
    private static bool DiversionSentence(DataRow senRow)
    {
        return Api.ParseCode(senRow.Field<string>("SenType")).StartsWith("43");
    }

    /// <summary>
    /// These are predicates that test a sentence row to see if it should be updated
    /// if it lives under a charge disposed with the specified disposition type.
    /// 
    /// For instance, if the PDDispositionCode is 413320, any DEJ sentence under the
    /// charge should be updated.
    /// </summary>
    private static readonly Dictionary<string, Func<DataRow, bool>> PDSentenceTests = 
        new Dictionary<string, Func<DataRow, bool>>
    {
        {"411610", DiversionSentence},  // diversion successful
        {"413320", DEJSentence},        // DEJ successful
        {"442110", DiversionSentence},  // diversion unsuccessful
        {"442111", DiversionSentence},  // diversion unsuccessful
        {"442112", DiversionSentence},  // diversion unsuccessful
        {"442120", DEJSentence}         // DEJ unsuccessful
    };

    /// <summary>
    /// These are the update actions that are applied to the sentence rows which pass the
    /// sentence test for the specified disposition type.
    /// 
    /// For instance, if the PDDispositionCode is 442110, sentences that pass the sentence
    /// test should be terminated.
    /// </summary>
    private static readonly Dictionary<string, Action<DataRow, DateTime>> PDSentenceUpdates = 
        new Dictionary<string, Action<DataRow, DateTime>>
    {
        {"411610", CompleteSentence},   // diversion successful (completed)
        {"413320", CompleteSentence},   // DEJ successful (completed)
        {"442110", TerminateSentence},  // diversion unsuccessful (terminated)
        {"442111", TerminateSentence},  // diversion unsuccessful (terminated)
        {"442112", TerminateSentence},  // diversion unsuccessful (terminated)
        {"442120", TerminateSentence}   // DEJ unsuccessful (terminated)
    };

    private void PDUpdateSentencesFromNewDisposition()
    {
        foreach (DataRow chargeRow in PDChargeRows
            .Where(x => PDSentenceTests.ContainsKey(x.Field<string>("PDDispositionCode"))))
        {
            string disp = chargeRow.Field<string>("PDDispositionCode");
            foreach (DataRow s in CHGRows[chargeRow]
                .ChildRows("CAS-SUBCRM-CHG-SEN")
                .Where(x => PDSentenceTests[disp](x)))
            {
                PDSentenceUpdates[disp](s, EventDate);
            }
        }
    }
Robert Rossney
  • 94,622
  • 24
  • 146
  • 218