Return to Answer

Since you're using WPF, you should really consider implementing MVVM.

Wouldn't Environment.Newline be better than "\r\n"?

Also, why do you add the new line at those times? That way you're intorducing several points where you could forget to do so. Wouldn't it be better to apply this only in AssignTextToTextBox(), e.g. Text = text + Environment.Newline?

Comments should explain why something was implemented in that way, not what the code is doing. For example: //If lines aren't the same, apply brush doesn't tell me anything the code isn't showing me already. If you need comments to explain what your code does, it's a sign you need to re-write the code.

I don't particularly see the point of text1Length and text2Length, just use text1.Count and text2.Count directly. But more importantly, why are you using StringCollection when List<string> is available?

If you apply these changes, I think your logic can be reduced to this:

var maximum = (list1.Count > list2.Count) ? list1.Count : list2.Count;
for (var i = 0; i < maximum; i++)
{
 var text1Line = (i >= list1.Count) ? string.Empty : list1[i];
 var text2Line = (i >= list2.Count) ? string.Empty : list2[i];
 var applicableBbrushColour = (text1Line == text2Line) ? null : brushColour;
 AssignTextToTextBox(richTextBox1, text1Line, applicableBbrushColour);
 AssignTextToTextBox(richTextBox2, text2Line, applicableBbrushColour);
}

You could even move (i >= list1.Count) ? string.Empty : list1[i]; to a separate method, considering this logic is repeated; perhaps even to an extension method?

Since you're using WPF, you should really consider implementing MVVM.

Wouldn't Environment.Newline be better than "\r\n"?

Also, why do you add the new line at those times? That way you're intorducing several points where you could forget to do so. Wouldn't it be better to apply this only in AssignTextToTextBox(), e.g. Text = text + Environment.Newline?

Comments should explain why something was implemented in that way, not what the code is doing. For example: //If lines aren't the same, apply brush doesn't tell me anything the code isn't showing me already. If you need comments to explain what your code does, it's a sign you need to re-write the code.

I don't particularly see the point of text1Length and text2Length, just use text1.Count and text2.Count directly. But more importantly, why are you using StringCollection when List<string> is available?

If you apply these changes, I think your logic can be reduced to this:

var maximum = (list1.Count > list2.Count) ? list1.Count : list2.Count;
for (var i = 0; i < maximum; i++)
{
 var text1Line = (i >= list1.Count) ? string.Empty : list1[i];
 var text2Line = (i >= list2.Count) ? string.Empty : list2[i];
 var applicableBbrushColour = (text1Line == text2Line) ? null : brushColour;
 AssignTextToTextBox(richTextBox1, text1Line, applicableBbrushColour);
 AssignTextToTextBox(richTextBox2, text2Line, applicableBbrushColour);
}

You could even move (i >= list1.Count) ? string.Empty : list1[i]; to a separate method, considering this logic is repeated; perhaps even to an extension method?

Since you're using WPF, you should really consider implementing MVVM.

Wouldn't Environment.Newline be better than "\r\n"?

Also, why do you add the new line at those times? That way you're intorducing several points where you could forget to do so. Wouldn't it be better to apply this only in AssignTextToTextBox(), e.g. Text = text + Environment.Newline?

Comments should explain why something was implemented in that way, not what the code is doing. For example: //If lines aren't the same, apply brush doesn't tell me anything the code isn't showing me already. If you need comments to explain what your code does, it's a sign you need to re-write the code.

I don't particularly see the point of text1Length and text2Length, just use text1.Count and text2.Count directly. But more importantly, why are you using StringCollection when List<string> is available?

Since you're using WPF, you should really consider implementing MVVM.

Wouldn't Environment.Newline be better than "\r\n"?

Also, why do you add the new line at those times? That way you're intorducing several points where you could forget to do so. Wouldn't it be better to apply this only in AssignTextToTextBox(), e.g. Text = text + Environment.Newline?

Comments should explain why something was implemented in that way, not what the code is doing. For example: //If lines aren't the same, apply brush doesn't tell me anything the code isn't showing me already. If you need comments to explain what your code does, it's a sign you need to re-write the code.

I don't particularly see the point of text1Length and text2Length, just use text1.Count and text2.Count directly. But more importantly, why are you using StringCollection when List<string> is available?

If you apply these changes, I think your logic can be reduced to this:

var maximum = (list1.Count > list2.Count) ? list1.Count : list2.Count;
for (var i = 0; i < maximum; i++)
{
 var text1Line = (i >= list1.Count) ? string.Empty : list1[i];
 var text2Line = (i >= list2.Count) ? string.Empty : list2[i];
 var applicableBbrushColour = (text1Line == text2Line) ? null : brushColour;
 AssignTextToTextBox(richTextBox1, text1Line, applicableBbrushColour);
 AssignTextToTextBox(richTextBox2, text2Line, applicableBbrushColour);
}

You could even move (i >= list1.Count) ? string.Empty : list1[i]; to a separate method, considering this logic is repeated; perhaps even to an extension method?