Replacing if else statement with any design pattern or better approach

Question

This code doesn't look clean and this if condition can grow

public int VisitMonth(int months)
    {
        int visit = 0;

        if (months <= 1)
        {
            visit = 1;
        }
        else if (months <= 2)
        {
            visit = 2;
        }
        else if (months <= 4)
        {
            visit = 3;
        }
        else if (months <= 6)
        {
            visit = 4;
        }
        else if (months <= 9)
        {
            visit = 5;
        }
        else if (months <= 12)
        {
            visit = 6;
        }
        else if (months <= 15)
        {
            visit = 7;
        }
        else if (months <= 18)
        {
            visit = 8;
        }
        else if (months <= 24)
        {
            visit = 9;
        }
        else if (months <= 30)
        {
            visit = 10;
        }
        else if (months <= 36)
        {
            visit = 11;
        }
        else if (months <= 48)
        {
            visit = 12;
        }
        else if (months <= 60)
        {
            visit = 13;
        }
        else
        {
            visit = 14;
        }
        return visit;
    }

Is there any better solution to this problem? Sadly that function isn't linear so it's not easy to code that in a mathematical way.

Answer 1

Should be more suitable to be reused :You can write a "Interval" Class with "inRange" methode like this :

 public struct Interval<T>
       where T : IComparable
{
    public T Start { get; set; }
    public T End { get; set; }
    public T Visit { get; set; }

    public Interval(T visit, T start, T end)
    {
        Visit = visit;
        Start = start;
        End = end;
    }

    public bool InRange(T value)
    {
      return ((!Start.HasValue || value.CompareTo(Start.Value) > 0) &&
          (!End.HasValue || End.Value.CompareTo(value) >= 0));
    }
}

And then use like this :

public static readonly List<Interval<int>> range = new List<Interval<int>>
        {
                new Interval<int>(1, 0, 1),
                new Interval<int>(2, 1, 2),
                new Interval<int>(3, 2, 4),
                new Interval<int>(4, 4, 6),
                new Interval<int>(5, 6, 9),
                new Interval<int>(6, 9, 12),
                new Interval<int>(7, 12, 15),
                new Interval<int>(8, 15, 18),
                new Interval<int>(9, 18, 24),
                new Interval<int>(10, 24, 30),
                new Interval<int>(11, 30, 36),
                new Interval<int>(12, 36, 48),
                new Interval<int>(13, 48, 60),
                new Interval<int>(14, 60, int.MaxValue)
        };

var months = 5;
var visit = range.Where(x => x.InRange(months)).Select(x => x.Visit).FirstOrDefault();

Answer 2

void Main()
{
    var conditionsChain = new SimpleCondition(0, 1);
        conditionsChain.AddNext(new SimpleCondition(1, 1))
        .AddNext(new SimpleCondition(2, 2))
        .AddNext(new SimpleCondition(4, 3))
        .AddNext(new SimpleCondition(6, 4))
        .AddNext(new SimpleCondition(9, 5))
        .AddNext(new SimpleCondition(12, 6))
        .AddNext(new SimpleCondition(15, 7))
        .AddNext(new SimpleCondition(18, 8))
        .AddNext(new SimpleCondition(24, 9))
        .AddNext(new SimpleCondition(30, 10))
        .AddNext(new SimpleCondition(36, 11))
        .AddNext(new SimpleCondition(48, 12))
        .AddNext(new SimpleCondition(60, 13))
        .AddNext(new SimpleCondition(14));

    for (int i = 0; i < 62; i++)
    {
        Console.WriteLine($"{i}: {conditionsChain.Evaluate(i) - VisitMonth(i)}");
    }
}

class SimpleCondition
{
    private SimpleCondition _next;

    private int _key;
    private int _result;

    public SimpleCondition(int key, int result)
    {
        _key = key;
        _result = result;
    }

    public SimpleCondition(int result) : this(-1, result)
    {
    }

    public int Evaluate(int key)
    {
        if(_key == -1)
        {
            return _result; 
        }

        if(key <= _key)
        {
            return _result;
        }
        else
        {
            if(_next == null)
            {
                throw new Exception("Default condition has not been configured.");
            }
            return _next.Evaluate(key); 
        }
    }

    public SimpleCondition AddNext(SimpleCondition next)
    {
        return _next = next;
    }
}

Answer 3

One way to do this would be to store the values you're comparing to in a List<int?>, and then return the index + 1 of the first item that meets the condition months <= item, or the list Count + 1 if none meet that condition.

We use int? to allow us to get a null result when we call FirstOrDefault if there are no matches (we don't use int, because default(int) == 0 so we wouldn't know if we matched the first item at index 0 or there were no matches and the default of 0 was returned). This way, we can test for a null result (meaning there is no match), and return List.Count + 1 in that case.

This reduces your method down to 2 lines of code, and adding a new condition like if (months <= 120) is as simple as adding a 120 to the values assignment:

public static int Visit(int months)
{
    var values = new List<int?> {1, 2, 4, 6, 9, 12, 15, 18, 24, 30, 36, 48, 60};

    return (values.Select((v, i) => new {value = v, index = i})
        .FirstOrDefault(i => months <= i.value)?.index ?? values.Count) + 1;
}

Answer 4

This will only work as long as the return value (visit) will always increase linearly for each available condition (i.e. visit increases by 1 in each if/else if block).

static readonly int[] _monthLimits = new int[] { 1, 2, 4, 6, 9, 12, 15, 18, 24, 30, 36, 48, 60 };

public static int VisitMonth(int months)
{
    int visit = 0;

    var maxMonths = _monthLimits[_monthLimits.Length - 1];
    if (months <= maxMonths)
    {
        // Only iterate through month limits if the given "months" is below the max available limit
        for (var i = 0; i < _monthLimits.Length; i++)
        {
            if (months <= _monthLimits[i])
            {
                visit = i + 1;
                break;
            }
        }
    }
    else
    {
        // The given "months" is over the max, default to the size of the array
        visit = _monthLimits.Length + 1;
    }

    return visit;
}

This approach has the benefit of not actually having to define what the return value (visit) is. Which makes this extensible in the sense that if a requirement ever comes in where there's a new limit somewhere in the middle (e.g. 22), you won't have to re-map the visit value for each subsequent condition since it is just derived based on its position in the array.

---

Here's an example of this working:

static void Main(string[] args)
{
    Console.WriteLine($"0: {VisitMonth(0)}");
    Console.WriteLine($"5: {VisitMonth(5)}");
    Console.WriteLine($"60: {VisitMonth(60)}");
    Console.WriteLine($"100: {VisitMonth(100)}");
    Console.ReadLine();
}

Answer 5

Possibly in C# 8 (this feature is not official yet, but works in recent IDEs if you turn it on):

int months = ...;
int visit = months switch
{
    int j when j <= 1 => 1,
    int j when j <= 2 => 2,
    int j when j <= 4 => 3,
    int j when j <= 6 => 4,
    int j when j <= 9 => 5,
    // ...
    _ => 42 // default
};

You can do similar in earlier C#, since this is a method:

public int VisitMonth(int months)
{
    switch (months)
    {
        case int j when j <= 1: return 1;
        case int j when j <= 2: return 2;
        case int j when j <= 4: return 3;
        // etc
        default: return 14;
    }
}

Answer 6

You can use a Dictionary to store the months as keys, and the visits as values.

var monthsToVisits= new Dictionary<int,int>
{
    {1,1},
    {2,2},
    {4,3},
    {6,4}
};

etc...

With this, you can easy look up the greatest key that is just more that the months you just want to check, and the associated value.

int months = 42;
int visit = monthsToVisits.Where(x => x.Key > months)
                        .OrderBy(x => x.Key)
                        .First().Value;

UPDATE

As @Marc Gravell said, using a dictionary is a highly inefficient solution. A better approach would be a static array.

static readonly (int Months,int Visit)[] monthsToVisits = new (int,int)[] 
{ 
    (1,1), 
    (2,2), 
    (4,3), 
    (6,4) 
};

public int VisitMonth(int months) => 
    monthsToVisits.First(x => months <= x.Months).Visit;