0

Given this example snippet:

if(test == 5) {
    var = 5;
    var2 = 6;
}
else if(test == 6){
    var = 30;
    var2 = 25;
}
//...ect

How can I clean this up into a function? I thought of doing this:

void doStuff(int condition, int v1, int v2){
    if(test == condition){
        var = v1;
        var2 = v2;
    }
}

but then I would have to implement it like this:

doStuff(5,5,6);
doStuff(6,30,25);
//...ect

This would go through each function and check each if statement even if the first was evaluated to be true. This would not have the if, else if, else function unless I did something like this: 

//Assuming doStuff returns a bool
if(doStuff(5,5,6)
else if(doStuff(6,30,25))
//...ect

Is there a better way to put functions inside conditional if / else if statements?

Improve this question
5
  • Without a less contrived example; its really hard to say. Both ways can be correct. Commented Nov 11, 2014 at 17:52
  • 1
    Personally I'd make the function handle the statment and not the condition. Commented Nov 11, 2014 at 17:53
  • None of this seems necessary. It's arguably more readable in its original form. If anything, the 5 and 6 look to the reader like magic numbers and should probably be placed in an enum or turned into constants. Commented Nov 11, 2014 at 17:57
  • @CÅdahl That's a good point. I will change my numbers to variables as they do look like magic numbers. I want to cut down on the amount of code because I have 14 different conditions and I have multiple statements inside each. I made the example so simple because I don't even have an idea of other techniques. Thanks for your input. Commented Nov 11, 2014 at 18:01
  • Well knowing that is also useful context. Are you always setting the same two results? Then maybe you could use an array or two instead. Then just index by 'test', cutting away most of the code. Commented Nov 11, 2014 at 18:04

3 Answers 3

Reset to default
1

The switch approach is probably the best, but if the number of cases is huge, you can consider doing something funny like...

 private static readonly Dictionary<int, int[]> _dic = new...

 private void Foo()
 {
     var test = ...
     var vals = this._dic[test];
     a = vals[0];
     b = vals[1];
     ...
 }

And if not all variables are of type int, you can either use Tuple or your own structure to hold the information (+1 for naming values)

Edit: regarding your updated question about "better way to use methods in if-else":

doStuff(5,5,6) || doStuff(6,30,25) || ...

Will evaluate doStuff's from left to right until one returns true.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Wow this is great. I hadn't thought about using a dictionary nor using OR! I will look into those. Thanks.
Crap! You beat me to it.
@ScottNimrod Lol didn't refresh in 13 minutes? >.> I always click the "load new answers" button before starting my own.
0 
var conditionsMap = new Dictionary<int, Tuple<int, int>>();
conditionsMap.Add(5,  new Tuple<int, int>(5, 6));
conditionsMap.Add(6, new Tuple<int, int>(30, 25));

foreach (var entry in conditionsMap)
{
    var key = entry.Key;
    var var1 = entry.Value.Item1;
    var var2 = entry.Value.Item2;

    Console.WriteLine("{0}\n{1}\n{3}", key, var1, var2);
}
Improve this answer

Comments

0

In these situations, I tend to use an enum with custom attributes to define the different conditions. To me, that keeps it concise which aids in maintenance and also helps restrict the possible inputs to a predefined set.

Here's a full code sample (it can be pasted into LINQPad to test):

public class ValuesAttribute : Attribute
{
    public int V1 { get; private set; }
    public int V2 { get; private set; }

    public ValuesAttribute(int v1, int v2)
    {
        V1 = v1;
        V2 = v2;
    }
}

public enum ValuesCase
{
    [Values(5, 6)]
    Five,
    [Values(30, 25)]
    Six
}

public void DoStuff(ValuesCase valuesCase)
{
    // There are several ways to get an attribute value of an enum, but I like this one
    ValuesAttribute values = valuesCase
        .GetType()
        .GetTypeInfo()
        .GetDeclaredField(valuesCase.ToString())
        .GetCustomAttribute<ValuesAttribute>();

    if(values != null)
    {
        // Assign your variables or do whatever else you want here
        Console.WriteLine(string.Join(", ", valuesCase.ToString(), values.V1, values.V2));
    }
}

void Main()
{
    DoStuff(ValuesCase.Five);
    DoStuff(ValuesCase.Six);
}
Improve this answer

1 Comment

I had just finished implementing Yorye's solution and will work wonderfully for me, I do however like the readability of this and will definitely keep in mind for the future. Many thanks!

Your Answer

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