What NOT to do in .NET Development


When I am frustrated, I like to blog. In fact, most of my best blog posts have been about something that frustrates me. Currently, I am frustrated with the MapQuest API, but not because of the API as much as trying to lay maps on a site that was developed a while back that follows some practices that one should not follow. In order to save you some headache, I am passing on my wisdom.

Do Not Loop Through Data to Create Reports

I am starting with this one, as many I have said this to say "well, duh!". Unfortunately, it is not always "well, duh!" as Microsoft used a looping model in ASP. Once you make the paradigm shift to data binding, you no longer need to loop to bind data. Here is a sample, from the app:

string s = "<table style="font-size:11px;" width="100%" border="0" cellspacing="0" cellpadding="3">";
s += "<tr bgcolor="#293f34"><th style="color:#FFFFFF">Start</th><th style="color:#FFFFFF">Stop</th><th style="color:#FFFFFF">Duration</th><th style="color:#FFFFFF">Address</th><th style="color:#FFFFFF">Event</th><th style="color:#FFFFFF">Latitude</th><th style="color:#FFFFFF">Longitude</th></tr>";

foreach (Report r in R)
{
    //Hundreds more concats here to create individual rows
}

What is happening here is a DataSet is being treated as if it were an ADO Recordset (remember – rs = CreatObject("ADODB.Recordset")?). In ado, it was common to loop and output. In .NET, you can use a simpler method:

GridView1.DataSource = R;

Wow, from thousands of lines to one. And the speed difference will blow your socks off. Wow, that guy who created the app must be a moron? Not actually, he ran into an issue. The customer can dictate which columns are included in the report, so he was looping so he could make column decisions, like (pseudocode here, as I am too lazy to search 1000s of lines of code or look this up):

if(User.Columns.Contains["NumberOfSats"])
{
    //Code to include this column
}

Is there a better way to do this? I can think of multiple ways, but here are some ways to solve other than hammering your web server with concats. Note that some of these will only solve one problem and some are not good ideas in some situations. But, here are some things we can do:

  1. Use a StringBuilder – see "Do Not Concatenate Strings" – this does not really solve the problem, but it sure speeds up the bad code
  2. Write dynamic SQL code – This is generally overkill, but pruning your result set at the database is a great way to bind without having to alter your application (allows for lazy binding). This is by far the simplest on the .NET side, but can get quite complex on the SQLside. There is a way around SQL complexity, but it requires executing dynamic SQL, which does not create statistics to improve SQL performance.
  3. Write your stored procedure as .NET code. This is better, in many than dynamic SQL code, but it can still get you to a place where the stats do not help much.
  4. Dynamically alter the bound columns on the GridView – This allows you to use the full result set every time. Only those columns chosen will be bound and no data loop.
  5. Prune the DataSet – Get rid of columns you are not going to bind. This allows you to be lazy on your GridView (not explicitly name columns) without having to loop through data

Of these choices, as you can probably guess, I would pick either 4 or 5. In most cases, option 4 would be the lightest on your server processor, so that would be where I would aim first. To accomplish this, add the GridView, rename it (unless you really like GridView1), and add the columns based on user prefs.

After typing this, I thought of one other option, or a variation on option 3, and that is to prune the result set in a .NET assembly in SQL Server. This is not an option if you are not using SQL Server, of course.

Do Not Concantenate Strings

This one flows from the last one, as the desire to loop data lead to the decision to concatenate. Consider the following:

class Program
{
    private static double Concat(int loopSize)
    {
        DateTime start = DateTime.Now;

        string s = String.Empty;
        for (int i = 0; i < loopSize; i++)
        {
            s += i.ToString();
            s += "rn";
        }

        DateTime end = DateTime.Now;

        return end.Ticks – start.Ticks;
    }

    private static double NoConcat(int loopSize)
    {
        DateTime start = DateTime.Now;

        StringBuilder builder = new StringBuilder();
        for (int i = 0; i < loopSize; i++)
        {
            builder.Append(i);
            builder.Append("rn");
        }

        DateTime end = DateTime.Now;

        return end.Ticks – start.Ticks;
    }

    static void Main()
    {
        int loopSize = 1000;
        int increment = 1000;

        Console.WriteLine("Loop size: {0}", loopSize);

        for(int i=0;i<10;i++)
        {
            double concat = Concat(loopSize);
            double noconcat = NoConcat(loopSize);

            Console.WriteLine("Comparison {0}", i+1);
            Console.WriteLine("String Concat: {0}", concat);
            Console.WriteLine("StringBuilder: {0}", noconcat);
            Console.WriteLine();

            loopSize += increment;
        }

        Console.Read();
    }
}

Forgive me for being lazy in my "counting" method. It is rather crude, but the differences between the two are pretty startling and I did not really want to think too much right now. If you run this, you will see some overhead popping up in the StringBuilder from time to time. If someone wants to write a better counter, go for it. My main point is adding lines to the StringBuilder does not affect performance much. Adding string concats does.

Am I a "Nazi" about this? Certainly not. If you have a bit of code where you are returning something simple, like "Hello" + name, I do not see a reason to hyperventilate at a concatenation. It makes the code more readable and the performance penalty is not enough to cry over. Once you start adding more than a few strings, you need to switch to a StringBuilder. And, if you are looping, it is imperative.

Don’t Throw New Technology in the Middle of a Build Cycle

This one is a pet peeve. Whenever Microsoft comes out with a new hammer, developers invariably start looking for a nail to fit the hammer. It does not matter that they are using screws. Perhaps this is a bad analogy, but you get the picture.

I am not saying you should shun new technology. There are certainly technologies that warrant the learning curve and throwing them in. But, you should be sure both of the following conditions are met:

  1. The technology is necessary to complete your application or (more likely) will GREATLY simplify your work.
  2. You have the time to learn how to do it right before implementing in a production application

Most often, the developer wants to learn something cool and figures he can stick it in the latest application. Since the development schedule is already hectic, he cannot find the time to properly learn the new technology, which leads to things like looping DataSets in ADO.NET. I will not talk about the AJAX applications I have seen lately. 🙂

The point here is you should make a strong business case before slapping new technology into an application, especially an application you are ALREADY working on. If you can make the case, then you fight for the time to learn it well enough you don’t do something ignorant or, worse, stupid. If you want to build your resume, play at home.

Don’t Reinvent the Wheel

Let me show you a security model. It is rather simple.

u = (ObsucatedLibraryName.User)Session["User"];
if (u == null)
    Response.Redirect("~/login.aspx");

Like it? If you have been developing in .NET very long, you probably deplore it about as much as I do. once again, this has its roots in ASP. Using this model, I have to get the User object on every single page, even if I am not using it for anything other than redirecting the user. I can make this less painful by moving these bits to a master page or a base page, but .NET makes this much simpler.

<authentication mode="Forms">
  <forms name="Something" loginUrl="login.aspx"/>
</authentication>

I can then set security on any page I want. In fact, I can use roles in the web.sitemap file, for my menus. Or, I can simply set up web.configs on different directories/files and kill both birds (menu and security) with one stone. And, I do not have to make an artificial call to Response.Redirect. In addition, I get a redirect back to the page I wanted to visit without me writing the mechanism.

The point here is simple. If there is a built in mechanism to do something, you better have a really good reason for writing your own. With security, there might be a case where you want to write your own, but you do not have to go gonzo custom to get it done, as there is a provider model.

In this app, I am retooling, there is:

  1. A custom security model (above)
  2. Hard coded menu items (which forces two master pages, simply to add administrative menus)
  3. Custom role mechanism (largely implemented in the two master pages)

I could probably go on, but each of these are handled with standard ASP.NET bits and do not require rewriting the entire system to get it the way we would like it to work in this application.

End Note

There are other things I could rant on about, but the above can be summed up in a few points (the first two being the same point, but from a slightly different perspective):

  • Know how to code in the paradigm you are coding in before you code in it. If you come from an ASP background, you will have to relearn if you do not want to suck.
  • Don’t play with new technology until you have time to learn it. If you are with a company that will not give you the time, you probably need to consider moving on.
  • When you have a complex problem, sit back and research before coding. If you do not, you will either end up in refactor hell or your application will suck.

While this may seem like I am railing on the developer in question, that is not my intent. I have found a great many developers who switched from the COM world who suck in .NET. Those who are intent on learning eventually get over these hurdles; those who are content continue to suck. Intent is better than content every day.

I need to write an entry on the "lazy programmers manifesto". Oh, that will have to wait. I have to get back to this application and figure out which CSS element is causing my map to puke. Until next time.

Peace and Grace,
Greg

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: