Nested Loop select the minimum defined value asp.net

Viewed 51

I have a list of states, which are defined to be ordered by min to max. the sequence is the following:

Cancelled - complete - draft - reservation - reserved - ordered - confirmed

So the cancelled is the minimum state, and confirmed is the maximum state. I may have different instances with different states, so I use a for-each loop to run through all states, and select the minimum state present in the loop.

That is: if in a list I have states [complete, reserved, draft, ordered] I need to check all the values and select complete -as it appears to be the minimum state. OR if I have [reserved, confirmed, ordered, draft, cancelled, confirmed, confirmed] I need to select the cancelled value, as it appears to be the minimum.

I am doing the following check, but it does not seem to be working:

        string globstatus = " ";
        foreach (var currentstatus in list)
        {
            if (currentstatus == "cancelled")
            {
                globstatus = "cancelled";
            }
            else
            {
                if (globstatus == "cancelled")
                {
                    return globstatus;
                }
                else
                {
                    if (currentstatus == "complete")
                    {
                        globstatus = "complete";
                    }
                    else
                    {
                        if (globstatus == "complete")
                        {
                            return globstatus;
                        }
                        else
                        {
                            if (currentstatus == "draft")
                            {
                                globstatus = "draft";
                            }
                            else
                            {
                                if (globstatus == "reservation")
                                {
                                    return globstatus;
                                }
                                else
                                {
                                    if (currentstatus == "reserved")
                                    {
                                        globstatus = "reserved";
                                    }
                                    else
                                    {
                                        if (globstatus == "ordered")
                                        {
                                            return globstatus;
                                        }
                                        else
                                        {
                                            if (currentstatus == "confirmed")
                                            {
                                                globstatus = "confirmed";
                                            }
                                            else
                                            {
                                                return currentstatus;
                                            }
                                        }
                                    }
                                }
                            }
                        }

                    }
                }
            }
        }

        return globstatus;

What can be the best solution to achieve the desired behavior?

1 Answers

I find a rule of thumb helpful that if I need more than three levels of braces, I need to rethink my code. It's hard to follow, easy to make mistakes, and a nightmare to debug. I suggest that applies here - trying to follow the flow of what all those nested if..else statements is extremely difficult.

Using Enum

My preferred solution is to achieve this using an Enum, e.g.:

var list = new List<Status>
{
    Status.Complete,
    Status.Draft,
    Status.Draft,
    Status.Confirmed
};

var minStatus = (Status)list.Select(l => (int)l).Min();
// minStatus = Status.Complete

public enum Status
{
    Cancelled,
    Complete,
    Draft,
    Reservation,
    Reserved,
    Ordered,
    Confirmed
}

How it works: by default Enums give each value a zero-based integer, i.e. Cancelled = 0, Complete = 1 and so on. You can override this with your own values if you wish (e.g. 1/2/4/8/16 if you want to combine multiple values).

I recommend using Enum types for things like this, rather than strings. It helps avoid typos, gives someone else looking at your code a clear understanding of how your program works and its flow, and represents hierarchy in a way in which simple strings don't. (For example - does 'complete' come before or after 'draft'? Without context, I imagine most people would say after, but in this case it comes before - that is much more obvious when using an Enum.)

Parse strings to Enum

However if the statuses have to be strings, you could parse them into an enum like so:

var stringList = new List<string>
{
    "complete",
    "draft",
    "draft",
    "confirmed",
    "this will be ignored"
};

var statusList = new List<int>();
foreach (var str in stringList)
{
    if(Enum.TryParse(typeof(Status), str, ignoreCase: true, out object? parsed) && parsed is Status status)
    {
        statusList.Add((int)status);
    }
}

var minStatus = (Status)statusList.Min();
// minStatus = Status.Complete

However, if it's possible to refactor your code to use the Enum in the first place, that would be a better solution, and much quicker as parsing strings has an overhead that would be good to avoid.

Related