18
\$\begingroup\$

Is there a more elegant way of doing this? I really think this looks ugly.

private static string InsertMethodNameHere( bool hasCondition1, bool hasCondition, bool hasCondition3)
        {    
            if (!hasCondition1 && !hasCondition2 && !hasCondition3)
                return "0";

            if (hasCondition1 && !hasCondition2 && !hasCondition3)
                return "1";

            if (!hasCondition1 && hasCondition2 && !hasCondition3)
                return "2";

            if (hasCondition1 && hasCondition2 && !hasCondition3)
                return "3";

            if (!hasCondition1 && !hasCondition2 && hasCondition3)
                return "4";

            if (hasCondition1 && !hasCondition2 && hasCondition3)
                return "5";

            if (!hasCondition1 && hasCondition2 && hasCondition3)
                return "6";

            if (hasCondition1 && hasCondition2 && hasCondition3)
                return "7";

            throw new Exception("Unable to determine.");
}
\$\endgroup\$

closed as off-topic by Jamal Dec 28 '14 at 19:09

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Might I ask why you think it's ugly? It looks perfectly readable to me. \$\endgroup\$ – pgraham Jun 29 '12 at 0:40
  • \$\begingroup\$ I suppose it is readable but, I didn't like so many returns within the method. \$\endgroup\$ – Tom Alderman Jul 7 '12 at 12:37
  • 1
    \$\begingroup\$ I guess it's a matter of preference but it's important to note that beauty isn't typically an aspect of software quality. I personally prefer the original to the accepted answer in terms of readability. \$\endgroup\$ – pgraham Jul 10 '12 at 15:26
26
\$\begingroup\$

How about this?

private static string InsertMethodNameHere(bool hasCondition1, bool hasCondition2, bool hasCondition3)
{
    return ((hasCondition1 ? 1 : 0) +
            (hasCondition2 ? 2 : 0) +
            (hasCondition3 ? 4 : 0)).ToString(CultureInfo.InvariantCulture);
}

If you want a more general solution, you could write an extension method that converts a list of bools to an int:

public static int ToInt(this IEnumerable<bool> bools)
{
    return bools.Select((t, i) => (t ? 1 : 0) << i).Sum();
}
\$\endgroup\$
  • \$\begingroup\$ Nice, very neat and readable. \$\endgroup\$ – Nadir Sampaoli Jun 26 '12 at 19:40
  • \$\begingroup\$ I like this. It looks clean and simple. Thanks! \$\endgroup\$ – Tom Alderman Jun 27 '12 at 11:17
  • 3
    \$\begingroup\$ Since my answer's all the way at the bottom and this is a good answer, just one thing; you could use the "params" keyword and make bools an array, and then the user wouldn't have to set up an actual IEnumerable of bools. You could set it up as an overload of this one, then you could turn a list of bools or a set of disparate variables into an int. \$\endgroup\$ – KeithS Jul 13 '12 at 15:10
21
\$\begingroup\$

You are representing some state using 3 individual boolean variables. It may be better to represent such a state using an enum with flags. Then this complicated test to figure what state you're in wouldn't have to be done. You could then combine flags to represent each of the separate states then switch off of that.

[Flags]
public enum MyState
{
    // your flags
    Default = 0x00,
    Condition1 = 0x01,
    Condition2 = 0x02,
    Condition3 = 0x04,

    // your state
    State0 = Default,
    State1 = Condition1,
    State2 = Condition2,
    State3 = Condition3,
    State4 = Condition1 | Condition2,
    State5 = Condition1 | Condition3,
    State6 = Condition2 | Condition3,
    State7 = Condition1 | Condition2 | Condition3,
}
private static void DispatchMethod(MyState state)
{
    switch (state)
    {
    case MyState.State0:
        // do something for state 0
        break;
    case MyState.State1:
        // do something for state 1
        break;
    case MyState.State2:
        // do something for state 2
        break;
    case MyState.State3:
        // do something for state 3
        break;
    case MyState.State4:
        // do something for state 4
        break;
    case MyState.State5:
        // do something for state 5
        break;
    case MyState.State6:
        // do something for state 6
        break;
    case MyState.State7:
        // do something for state 7
        break;
    }
}

You could still set and clear individual flags using simple bitwise logic.

state |= MyState.Condition1;  // sets condition 1
state &= ~MyState.Condition3; // clears condition 3

You could even wrap this up in a library to make doing these operations more intuitive.

\$\endgroup\$
  • \$\begingroup\$ Just a warning, calling ToString() on such an enum will not yield what you would probably expect. Since there are overlapping values, the default implementation will return possibly multiple values that applies. \$\endgroup\$ – Jeff Mercado Jun 26 '12 at 14:04
  • \$\begingroup\$ perhaps it would be better to put the 'flags' into a separate enum in order to avoid the overlapping ToString()? \$\endgroup\$ – Ben Cottrell Jun 26 '12 at 17:48
  • \$\begingroup\$ I considered that but the problem is that you would have to map out the values so that they would correspond between the types. But you don't necessarily need the result of what ToString() returns anyway, you can get the name of the value through other means. And in practice, I don't think there's much need for the string representation anyway, we only care about the value. \$\endgroup\$ – Jeff Mercado Jun 26 '12 at 18:11
5
\$\begingroup\$

Using the MyState flag enum from Jeff's answer you can do the method in Scroog1's answer like so:

private static string InsertMethodNameHere(bool hasCondition1, bool hasCondition2, bool hasCondition3)
{
    var state = MyState.Default;
    if(hasCondition1) state |= MyState.Condition1;
    if(hasCondition2) state |= MyState.Condition2;
    if(hasCondition3) state |= MyState.Condition3;
    return ((int)state).ToString(CultureInfo.InvariantCulture);
}

(the int cast is there to match your initial return state)

see also: http://weblogs.sqlteam.com/mladenp/archive/2007/01/12/57541.aspx

\$\endgroup\$
4
\$\begingroup\$
private static string InsertMethodNameHere(params bool[] conditions)
{
   conditions.Select((x,i)=>Convert.ToByte(x) << i).Sum().ToString();
}

In your particular situation, this would drop right in, which IMO makes it better than Scroog's because his requires the user to put the conditions into IEnumerable format. The params keyword is beautiful that way. This function (like many answers) assumes that the desired State will always be the concatenation of a big-endian bit array of the conditions, in order.

\$\endgroup\$

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