So I was attempting to find a bug in the following code (which was part of a for loop):
Label startLabel = new Label();
startLabel.Text = "<div class=" + ((counter % 2 == 0) ? columnTwo : columnOne) + "><h2><label for=\"" +
thisType.Name + "List\">" + thisType.FriendlyName + ":</label></h2>";
if (counter % 2 != 0) this.Dynamic.Controls.Add(new LiteralControl("<div class=\"searchContainer\">"));
this.Dynamic.Controls.Add(startLabel);
this.Dynamic.Controls.Add(sample);
Label endLabel = new Label();
endLabel.Text = "</div>";
this.Dynamic.Controls.Add(endLabel);
if ((counter % 2 == 0) || (counter % 2 != 0 && (counter - 1) == TypesInGroup.Count)) this.Dynamic.Controls.Add(new LiteralControl("</div>"));
counter++;
I found this really confusing and couldn't work out what was causing the bug. So I did some refactoring and tried to tidy it up a bit, in the process I found a couple of redundancies and two bugs! Below is the refactored code; notice that the main differences are formatting, variable names and simplifying the string concat by using String.Format.
string startHtml = "<div class=\"col1\"><h2><label for=\"{0}List\">{1}:</label></h2>";
LiteralControl startLabel = new LiteralControl(String.Format(startHtml, thisType.Name, thisType.FriendlyName));
LiteralControl endLabel = new LiteralControl("</div>");
bool isAlternateRow = (counter % 2 == 0);
if (!isAlternateRow)
this.Dynamic.Controls.Add(new LiteralControl("<div class=\"searchContainer\">"));
this.Dynamic.Controls.Add(startLabel);
this.Dynamic.Controls.Add(checkBoxList);
this.Dynamic.Controls.Add(endLabel);
bool isLast = (counter == this.SearchTypesList.Count);
if (isAlternateRow || isLast)
this.Dynamic.Controls.Add(new LiteralControl("</div>"));
counter++;
I find the second infinitely more readable! Lesson learned (again): the main aim of writing code is not to get it working (anyone can do that) but to make it easy to read for the next guy!
Barry