How can I make these if statement better?

Question

I am very new to C# and I have a question. I didn't find the right answer on google but how can I simplify or clean up this code.

    if (availableTables.Any(item => item.Name == "Table1") == false)
    {
        ViewBag.Table1FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table2") == false)
    {
        ViewBag.Table2FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table3") == false)
    {
        ViewBag.Table3FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table4") == false)
    {
        ViewBag.Table4FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table5") == false)
    {
        ViewBag.Table5FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table6") == false)
    {
        ViewBag.Table6FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table7") == false)
    {
        ViewBag.Table7FillColor = "#ff1200";
    }
    if (availableTables.Any(item => item.Name == "Table8") == false)
    {
        ViewBag.Table8FillColor = "#ff1200";
    }

You can optimize execution by using if/else so you don't execute all the ifs unnecessarily. — nicomp, Nov 11 '19 at 16:09
Ok, thnx nicomp. And the problem is that I want to have my code so clean as possible @DavidG . I don't want to learn the wrong ways of coding. — Bart Brouwers, Nov 11 '19 at 16:11

score 4 · Answer 1 · answered Nov 11 '19 at 16:14

Setting values in ViewData is effectively the same as dynamic properties in ViewBag (see this answer). This allows for a nice solution using arrays:

string[] tableNames = new string[]{"Table1", "Table2", ...};
string[] tableColorProperties = new string[]{"Table1FillColor", "Table2FillColor", ...};

for(int i=0; i<tableNames.Length; i++)
{
    if (availableTables.Any(item => item.Name == tableNames[i]) == false)
    {
        ViewData.Add(tableColorProperties[i], "#ff1200");
    }
}

This can be improved further based on personal taste: use dictionary instead of two arrays, LINQ, so on.

I have to say though that you should probably consider refactoring your controller-view data passing to use strongly typed models.

score 0 · Answer 2 · answered Nov 11 '19 at 17:05

If you ever have anything where there can be an arbitrary number of tables, you could match with Regex and add them accordingly:

var names = new List<string>
{
    "Table1", "SomeRandomName", "Table8907123", "Table123NoMatch"
};

var regex = new Regex(@"^Table[1-9]\d*$");

var matches = names.Select(s => regex.Match(s)).Where(match => match.Success);
foreach (var match in matches)
    ViewData.Add($"{match.Value}FillColor", "#ff1200");

And if you only want to match "Table1" to "Table8", you can use this Regex instead:

var regex = new Regex(@"^Table[1-8]$");

How can I make these if statement better?

2 Answers2