Refactoring a server side creating of JavaScript with C# verbatim literal strings (@)

Today I was refactoring a code of my job colleagues and stuck in horror (coding).

Beware Coding Horror on the way….

In one method I found this code:

string summary = String.Empty;
int counter = 0;
int i = 0;

foreach (PoiMapData poi in listAllPoi)
{
  string name = poi.KindName.Replace(" ", "_");
  string lat = poi.Latitude.ToString().Replace(",", ".");
  string lon = poi.Longitude.ToString().Replace(",", ".");

  summary = Helper.GetLeadingHtml(poi.Summary, 150);
  summary = summary.Replace("'", "\\'");

  string title = poi.Title.Replace("'", "\\'");

  i = poi.Id;

  ScriptString += "\t" + @"var point" + i + @" = new GLatLng(" + lat + ", " + lon + ");\n" + "\t" +
                       @"var marker" + i + @" = createMarker(point" + i + ", '" +
                       @"<span class=""title"">" + @"<a href=""PoiDetail.aspx?poiID=" + poi.Id + @""">" + title + @"</span><br />" +
                       "', '" + @"<br /><span class=""summary"">" + summary + @"</span>" + "', " +
                       "icon" + poi.KindId + ", " + "'" + name + "');\n " + "\t" +
                       "map.addOverlay(marker" + i + ");\n" + "\t" +
                       "listaPOI[" + counter + "] = marker" + i + ";\n\n ";
  counter++;
}

Holy Hejlsberg! If I may say I think this screaming Concatenation explains how very, very, and super very wrong this code is, and this is not all, because this piece of code is in foreach loop so it would continue over and over and over again. Horror!

…And this is still not all! Forget about speed and benchmarking, and for a second think what is most important in code: maintainability and readability, woohooo, can you even read above code. It is totally cluttered with “+”, “\n”, \t”, “@”!

But all that code doing is script (JavaScript) with GoogleMaps markers of all Points of interest, so it can be represent on map. But problem is that this JavaScript building is done on sever-side, using database for getting all Points of interest (there is also a grouping by kind of points of interest, but never mind that, back to refactoring).

So what is the best way to format and build formated JavaScript in C# language? For me it is by using, so called, verbatim literal string. In C#, is used @ sign (“monkey” as many would say, but actually is “at” sign) to indicate the verbatim literal string.

So what a hella heck is verbatim literal string. According to MSDN:

By prefixing a string literal with “@”, you can insert line breaks and other special characters into a string, and the C# compiler will interpret everything between the open and closing quotation marks as the contents of that string. Note that quotation marks within the string need to be doubled, or else the compiler will assume it has hit the end of the string literal.

So now is easy to refactor above shown horror. First why there is variable i, i is very popular in for loops, indicating index, but here uses point of interest Id. Why?!!. Actually, I would rename counter to index, or maybe use a for loop here better and some naming of variables are totally wrong, but let it be, that can pass, we want to refactor this concatenation coding horror…

Stop! …
Refactoring time!

string summary;
ScriptString = String.Empty;
int counter = 0;

foreach (PoiMapData poi in listAllPoi)
{
  summary = Helper.GetLeadingHtml(poi.Summary, 150);
  summary = summary.Replace("'", "\\'");

  string script =
    @"{0}
      var point{1} = new GLatLng({2}, {3});
      var marker{1} = createMarker(point{1},
                                   '<span class=""title""><a href=""PoiDetail.aspx?poiID={1}"">{4}</span><br />',
                                   '<br /><span class=""summary"">{5}</span>',
                                   icon{6},
                                   '{7}');
       map.addOverlay(marker{1});
       listaPOI[{8}] = marker{1};";

  ScriptString = String.Format(script,
                               ScriptString,                               // {0}
                               poi.Id,                                     // {1}
                               poi.Latitude.ToString().Replace(",", "."),  // {2}
                               poi.Longitude.ToString().Replace(",", "."), // {3}
                               poi.Title.Replace("'", "\\'"),              // {4}
                               summary,                                    // {5}
                               poi.KindId,                                 // {6}
                               poi.KindName.Replace(" ", "_"),             // {7}
                               counter);                                   // {8}

  counter++;
}

• I’ve removed many local variables, I just don’t see no purpose for their existence, especially famous (i)?!!.
• Create a string that will hold formatting in a placeholders ({0},…).
• In String.Format, first is ScriptString which is simulating “ScriptString +=” concatenation.
• Rest are those needed for GoogleMaps marker(s).
• String.Format could be in one line of code but I put comments of what number placeholder has, only for easier reading.
• Verbatim literal strings keep all formatting so there is no need for tabs, newlines, etc..

Even though this will produce “fat” (full of white-spaces) script, this code should be only used for development, and for deployment I would rather crate one flat script probably. Done preferably with StringBuilder.

Here are some nice blog post of prominent bloggers I’ve found while refactoring this coding horror:

• Jeff Atwood – The Sad Tragedy of Micro-Optimization Theater (Actually Jeff’s blog name is “Coding Horror”)
• Rico Mariani – Performance Quiz 1

Happy coding and refactor your ugly code every now and then.

It’s good for the soul!