Multiple values against same condition

Is there any other way to optimize the below code? I feel the below code is huge for the operations it performs.

{
    if ((currentElement == null ||
        (firstGridRow["Low"].ToString() == string.Empty ||
        firstGridRow["High"].ToString() == string.Empty ||
        firstGridRow["Mean"].ToString() == string.Empty ||
        firstGridRow["StdDev"].ToString() == string.Empty)))
    {
        continue;
    }

    double currentLow = Convert.ToDouble(firstGridRow["Low"]);
    double currentHigh = Convert.ToDouble(firstGridRow["High"]);
    double currentMean = Convert.ToDouble(firstGridRow["Mean"]);
    double currentStdDev = Convert.ToDouble(firstGridRow["StdDev"]);
    if (newRow.Length != 0)
    {
        AddColorList(currentElement, opid, currentLow, "Low", newRow, listCollectionLow);
        AddColorList(currentElement, opid, currentHigh, "High", newRow, listCollectionHigh);
        AddColorList(currentElement, opid, currentMean, "Mean", newRow, listCollectionMean);
        AddColorList(currentElement, opid, currentStdDev, "StdDev", newRow, listCollectionStdDev);
    }
}

So I’ll preface this with it’s a little difficult without the actual context of the code, and that all suggestions need to be checked to ensure they don’t break the existing flow of control.

A few options:

• Breaking out repetitive operations into reusable methods
• Wrapper methods (within reason) to apply some common operations and call some other method

With the assumption that these are inside of a loop, and you’re not relying on an exception from the Convert.ToDouble() call to break out then you might consider some approaches like follows.

Simplified from provided example. Assuming additional trailing ‘}’ is not significant.

if (currentElement != null
    && !RowHasEmptyCells(firstGridRow, "Low", "High", "Mean", "StdDev")
    && newRow.Lenth != 0)

{
    AddColourListWithSourceValue(firstGridRow, newRow, "Low", currentElement, opid, listCollectionLow);
    AddColourListWithSourceValue(firstGridRow, newRow, "High", currentElement, opid, listCollectionHigh);
    AddColourListWithSourceValue(firstGridRow, newRow, "Mean", currentElement, opid, listCollectionMean);
    AddColourListWithSourceValue(firstGridRow, newRow, "StdDev", currentElement, opid, listCollectionStdDev);
}

Functions attempting to contain some of the previously duplicated logic:

//return true if any of the named cells in the row are 'empty'
bool RowHasEmptyCells(RowType row, params string[] cellNames)
{
    return cellNames.Any(cellName => row[cellName].ToString() == string.Empty);
}

//RowType is the type of your row object (DataGridRow?)
void AddColourListWithSourceValue(RowType sourceRow, RowType newRow, string cellName, object currentElement, object opid, object listCollection)
{
    double currentValue = Convert.ToDouble(sourceRow[cellName]);
    AddColourList(currentElement, opid, currentValue, cellName, newRow, listCollection);
}

For the above double.Parse or .TryParse might be the way to go for a string > double conversion. If using Parse you could remove the empty cells check and just catch a FormatException/ArgumentNullException

This can likely be improved upon, but I’ve tried to make the flow of the main part a bit easier to follow whilst keeping roughly the same logic.

Edit 1: Sorry – fixed typo. Also AddColourListWithSourceValue() is just a guess at what’s going on 🙂

Edit 2: Quick edit with @svick’s correction on logic and suggestion for LINQ to check for empty. Inverted to allow for the positive method prefix ‘Has’ instead of a ‘CellsNotEmpty’. Would welcome thoughts on !IsSomething() vs IsNotSomething()…

Edit 3: != to == in RowHasEmptyCells() as pointed out by @svick