Verify which columns have changed in a datatable or datarow

Question

I was asked to update a business rule to allow a specific column to be added. Because our datalayer uses only DataSets as an interface to the database that is what we work with in the business layer.

Below I have created a couple of DataTable and DataRow extensions to use for this businessrule and I would like your opinion on things I might have missed. This is the method I use to search for changes

private static bool hasColumnChanged(StringComparison stringComparison, bool ignoreWhitespace, DataRow row, DataColumn col)
{
    bool isEqual = true;
    if (row[col, DataRowVersion.Original] != DBNull.Value && row[col, DataRowVersion.Current] != DBNull.Value)
    {
        if (ignoreWhitespace)
            isEqual = row[col, DataRowVersion.Original].ToString().Trim().Equals(row[col, DataRowVersion.Current].ToString().Trim(), stringComparison);
        else
            isEqual = row[col, DataRowVersion.Original].ToString().Equals(row[col, DataRowVersion.Current].ToString(), stringComparison);
    }
    else
        isEqual = row[col, DataRowVersion.Original].Equals(row[col, DataRowVersion.Current]);

    return !isEqual;
}

And these are simply the extensions using the code:

public static List<DataColumn> GetChangedColumns(this DataTable table) 
{
    return table.GetChangedColumns(StringComparison.InvariantCultureIgnoreCase, false);
}
public static List<DataColumn> GetChangedColumns(this DataTable table, bool ignoreWhitespace)
{
    return table.GetChangedColumns(StringComparison.InvariantCultureIgnoreCase, ignoreWhitespace);
}
public static List<DataColumn> GetChangedColumns(this DataTable table, StringComparison stringComparison, bool ignoreWhitespace)
{
    if (table == null) throw new ArgumentNullException("table");

    List<DataColumn> columnsChanged = new List<DataColumn>();
    foreach (DataRow row in table.GetChanges().Rows)
    {
        foreach (DataColumn col in row.Table.Columns)
        {
            if (!columnsChanged.Contains(col) && hasColumnChanged(stringComparison, ignoreWhitespace, row, col))
                columnsChanged.Add(col);
        }
    }
    return columnsChanged;
}

public static List<DataColumn> GetChangedColumns(this DataRow row)
{
    return row.GetChangedColumns(StringComparison.InvariantCultureIgnoreCase, false);
}
public static List<DataColumn> GetChangedColumns(this DataRow row, bool ignoreWhitespace)
{
    return row.GetChangedColumns(StringComparison.InvariantCultureIgnoreCase, ignoreWhitespace);
}
public static List<DataColumn> GetChangedColumns(this DataRow row, StringComparison stringComparison, bool ignoreWhitespace)
{
    if (row == null) throw new ArgumentNullException("row");

    List<DataColumn> columnsChanged = new List<DataColumn>();
    foreach (DataColumn col in row.Table.Columns)
    {
        if (!columnsChanged.Contains(col) && hasColumnChanged(stringComparison, ignoreWhitespace, row, col))
            columnsChanged.Add(col);
    }
    return columnsChanged;
}

To test the above code I use this simple unit test:

[TestMethod]
public void DataTableAndDataRowGetChangedColumns()
{
    DataSet ds = GetDummyDataSet();
    ds.Tables[0].Rows[0][3] = DateTime.Now;
    ds.Tables[0].Rows[0][2] = ds.Tables[0].Rows[1][2].ToString() + " ";
    ds.Tables[0].Rows[0][1] = DBNull.Value;
    List<DataColumn> changesForRow = ds.Tables[0].Rows[0].GetChangedColumns(true);
    List<DataColumn> changesForTable = ds.Tables[0].GetChangedColumns(true);

    // For now we just verify if the amount of changes is the same (ez way out)
    Assert.IsTrue(changesForRow.Count.Equals(changesForTable.Count));
}

In the business rule I have implemented this piece of code which verifies if there are other datacolumns that have changed:

List<DataColumn> columnsChanged = dsChanges.Tables[0].GetChangedColumns(true);
if(columnsChanged.Any(c=>!c.ColumnName.Equals("DateUntill", StringComparison.InvariantCultureIgnoreCase)))
    throw new BusinessException("This premium can not be changed, only DateUntill can still change");

Thank you very much for taking the time to read through my code!

Answer 1

1) hasColumnChanged method. Inside first if you have two almost same lines. Duplicated code should be extracted:

if (row[col, DataRowVersion.Original] != DBNull.Value && row[col, DataRowVersion.Current] != DBNull.Value)  
{  
    string originalVersionToCompare = row[col, DataRowVersion.Original].ToString();
    string currentVersionToCompare = row[col, DataRowVersion.Current].ToString();
    if (ignoreWhitespace)
    {
        originalVersionToCompare = originalVersionToCompare.Trim();
        currentVersionToCompare = currentVersionToCompare.Trim();
    }
    isEqual = originalVersionToCompare.Equals(currentVersionToCompare, stringComparison);
}    

2) If you have .Net 4.0 then 6 GetChangedColumns methods I would refactor into 2 with optional parameters.

3) foreach inside GetChangedColumns for DataRow looks like a copypaste. I cannot imagine a situation when columnsChanged.Contains(col) will be true in this method.

4) hasColumnChanged seems to be named incorrectly. It should be hasCellChanged since it checks for intersection of row and column. Maybe also PascalCase?

5) LINQify it! Original:

List<DataColumn> columnsChanged = new List<DataColumn>();
foreach (DataRow row in table.GetChanges().Rows)
{
    foreach (DataColumn col in row.Table.Columns)
    {
        if (!columnsChanged.Contains(col) && hasColumnChanged(stringComparison, ignoreWhitespace, row, col))
            columnsChanged.Add(col);
    }
}
return columnsChanged;

Result:

return table.GetChanges().Rows.Cast<DataRow>()
     .SelectMany(dr => table.Columns.Cast<DataColumn>(), (row, column) => new {row, column})
     .Where(c => hasColumnChanged(stringComparison, ignoreWhitespace, c.row, c.column))
     .Select(c => c.column)
     .Distinct()
     .ToList();

6) DateUntill double L?

7) Why do you compare items by casting them to string? Why don't you compare them as objects?