Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
for (int i = 0; i < keyList.Count; i++)
{
    if (oldDic.ContainsKey(keyList[i].ToString()))
    {
        if (newDic[keyList[i].ToString()].ToString() == oldDic[keyList[i].ToString()].ToString())
        {
            //ReminderBackupLog("Same");
        }
        else
        {
            isChnagedSectionFields = "Yes";
            string oldValue = oldDic[keyList[i].ToString()].ToString();
            string newValue = newDic[keyList[i].ToString()].ToString();
            table = table + "<tr style='border: 1px solid black;'><td style='border: 1px solid black;'>" + colNames[keyList[i].ToString()].ToString() + "</td><td style='border: 1px solid black;'>" + oldValue + "</td><td style='border: 1px solid black;'>" + newValue + "</td></tr>";
        }
    }
    else
    {
        isChnagedSectionFields = "Yes";
        string newValues = newDic[keyList[i].ToString()].ToString();
        table = table + "<tr style='border: 1px solid black;'><td style='border: 1px solid black;'>" + colNames[keyList[i].ToString()].ToString() + "</td><td style='border: 1px solid black;'>" + " " + "</td><td style='border: 1px solid black;'>" + newValues + "</td></tr>";
    }
}
share|improve this question
2  
What about deleted keys? Do you care about those? –  200_success May 18 at 9:49
    

2 Answers 2

You should separate the logic from your representation and first collect those differences in a separate list. Also, what are the types of your dictionaries? All those ToString()s make it really noisy and are likely not necessary. Furthermore, use a foreach loop to iterate through your key list, this will also automatically get rid of all those keyList[i] accesses. And finally, instead of appending to a string directly, use a StringBuilder to make it more efficient:

// Use a separate type to collect your diffs
class DiffEntry
{
    public string ColumnName { get; set; }
    public string OldValue { get; set; }
    public string NewValue { get; set; }
}

// collect all diffs
List<DiffEntry> diffs = new List<DiffEntry>();
foreach (string key in keyList)
{
    if (oldDic.ContainsKey(key))
    {
        if (newDic[key] != oldDic[key])
        {
            diffs.Add(new DiffEntry()
            {
                ColumnName = colNames[key],
                OldValue = oldDic[key],
                newValue = newDic[key]
            });
        }
    }
    else
    {
        diffs.Add(new DiffEntry()
        {
            ColumnName = newDic,
            NewValue = newDic[key]
        });
    }
}

// generate the actual table output
StringBuilder tableContent = new StringBuilder();
foreach (DiffEntry diff in diffs)
{
    tableContent.Append("<tr style='border: 1px solid black;'>");
    tableContent.Append("<td style='border: 1px solid black;'>");
    tableContent.Append(diff.ColumnName);
    tableContent.Append("</td><td style='border: 1px solid black;'>");
    tableContent.Append(diff.OldValue);
    tableContent.Append("</td><td style='border: 1px solid black;'>");
    tableContent.Append(diff.NewValue);
    tableContent.Append("</td></tr>");
}
table += tableContent.ToString();
share|improve this answer
    
type of your dictionaries is ToString() –  user73801 May 18 at 11:02
    
@poke May I suggest an improvement to DiffEntry class. You can add an enum ChangeAction to group the differences e.g. if the matching entry are different or an entirely new entry is there in newDic. –  Sandeep May 18 at 12:52

How about a generic solution using LINQ?

private string GetDiffTableRows<TKey, TValue>(IDictionary<TKey, TValue> oldValues, IDictionary<TKey, TValue> newValues)
{
    TValue oldValue, newValue;
    var diffs = oldValues.Keys.Concat(newValues.Keys).Distinct().Select(k => new
    {
        Key = k,
        OldHasKey = oldValues.TryGetValue(k, out oldValue),
        NewHasKey = newValues.TryGetValue(k, out newValue),
        OldValue = oldValue,
        NewValue = newValue
    }).Where(d => d.OldHasKey != d.NewHasKey || !Equals(d.OldValue, d.NewValue));

    var tableRows = diffs.Select(d => string.Format(DiffTableRowFormatString, d.Key, d.OldValue, d.NewValue));
    return string.Join(string.Empty, tableRows);
}

private const string DiffTableRowFormatString =
    "<tr style='border: 1px solid black;'>" +
        "<td style='border: 1px solid black;'>{0}</td>" +
        "<td style='border: 1px solid black;'>{1}</td>" +
        "<td style='border: 1px solid black;'>{2}</td>" +
    "</tr>";
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.