DaedTech

Stories about Software

By

Static and New Are Like Inline

C++ Inline

Reaching back into my C++ days, a concept exists called “inline”. “Suggesting inline” is a concept where you tell the C++ compiler that you want to dispense with function call overhead and slam the code in question right into the code stream of the caller. (It’s a matter of suggesting because the compiler might ignore this request during its optimization such as if you decide to rip a hole in space-time by suggesting inline on a recursive method). So, for instance:


inline int multiply(int x, int y)
{
     return x*y;
}

int main(int argc, char** argv)
{
     int product = multiply(2,5);
}

is effectively transformed into:

int main(int argc, char** argv)
{
     int product = 2*5;
}

This is conceptually similar to the concept of macros in C/C++. The idea is simple — you may have some block of code that you want to abstract out for reuse or readability, but you’d prefer the performance to mimick what would happen if you just wrote the code right in the method in question. You want to have your cake and to eat it too. Understandable — I mean, who doesn’t and what’s the point of having cake if you can’t eat it?

In the .NET world of managed languages, we don’t have this concept. It wouldn’t make any sense anyway as our builds generate byte code, which is processor-agnostic. The actual object code is going to be a matter between the runtime and the OS. So we don’t see the option for inline in the sense of runtime performance.

Metrics in OOP

One of the metrics in OOP that I think is a generally fair one is lines of code as a liability. If you have a 20-100 line class then life is good, but as you start creeping toward 300 lines it starts to smell. If it’s over 500 lines, kill it with fire (or, break it up into reasonable classes). The reason for this is a nod toward the Single Responsibility Principle and the concept of code smell. There’s nothing inherently wrong with large classes, per se, but they’re almost always an indication that a bunch of responsibilities are all knotted together in one tightly coupled mess. The same goes for methods on a smaller scale and with smaller scope of responsibility.

So, your personal preferences (and mine) about class/method size notwithstanding, it bears mentioning that smaller and more focused is generally considered better. The result of this is that it’s fairly common to evaluate class and method sizes with fellow developers and see people getting antsy as sizes get larger. On the flip side, it’s common to see people feel good when they keep class sizes small and to feel particularly good when they slay some hulking 5000 line beast and see it divided up into 20 more reasonably sized classes.

Another fairly common metric that I like is the number of parameters passed to a method. Like lines of code and large method/class, parameters trend toward code smell. No parameters is great, one is fine, two is a little noisy, and three is pushing it. More than that and you’ve written a method I want nothing to do with. I think a lot of people feel this way in terms of code they write and almost everyone feels this way when they’re a client (if a method has a bunch of parameters, good luck figuring out how to use it). This is true not only because it’s hard to keep all of the parameters straight but also because a method that needs a whole bunch of things to do its job is likely doing too large a job or too many jobs.

We like our methods/classes small and our parameter lists short.

Static and New: Gaming the System

One way to accomplish these goals is to ensure that your scopes are well defined, factored, and focused. But, if you don’t feel like doing that, you can cheat. If, for instance, you come across a constructor that takes 5 dependency parameters, the best thing to do would be to rework the object graph to have a more cohesive class that needed fewer dependencies. But, if time is short, you can just kick the pile of debris under the bed by having the constructor reach out into the static universe and pull its dependencies from the ether. Now your class has the cologne of a blissfully simple constructor hiding its dependnecy smell, thanks to some static or singleton access. The same thing can be applied with the “new” keyword. If you don’t want to rip holes in the fabric of your object graph with static state and functionality, you can always instantiate your own dependencies to keep that constructor looking slim (this would be identical in concept to having a stateless static factory method).

This trick also applies to cutting down lines of code. If you have a gigantic class, you could always port out some of its state and behavior out into a static class with static state or to an instance class spun up by the beast in question. In either case, lines of code goes down and arguments to public APIs stays the same.

Inline Revisited

Consider the following code:

public class Foo
{
    public int GetTotal(int n)
    {
        int total = 0;
        for (int index = 0; index < n; index++)
        {
            total += index;
        }
        return total;
    }
}

Let’s say that we thought that GetTotal method looked way too long and we wanted to shorten it by kicking parts of it under the bed when company came by to look at the class. What about this:

public class Foo
{
    public int GetTotal(int n)
    {
        return Utils.RunningSum(n);   
    }
}

This is fewer lines of code, to be sure. We’ve created a static Utils class that handles the dirty work of getting the running sum of all numbers up to and including a given number and we’ve delegated the work to this class. Not bad — we’ve eliminated 5 lines of code from both Foo and GetTotal(). And RunningSum is stateless, so there’s no worry about the kind of weird behavior and dependencies that you get from static state. This is a good thing, right?

Well, no, not really. I’d argue that this is fool’s gold in terms of our metrics. In a very real conceptual sense, Foo has no fewer lines of code than it initially did. Sure, from the perspective of organizing code it does — I’ve separated the concern of RunningSum from Foo’s GetTotal and we might make an argument that this factoring is a good thing (it’d be more interesting in a less trivial example). But from the perspective of coupling in the object graph, I’ve done exactly nothing.

When you call GetTotal(n), all of the same code is going to be executed in either case. All of the same branching will occur, all of the same logic will guide that branching, and all of the same local variables will be declared. From a dependency perspective as expressed in source code, you might as well inline Utils.RunningSum() into GetTotal(). To put this another way, you might as well conclude that Foo and GetTotal() are just as many lines of code as they ever were.

And that’s my larger point here. When your code invokes a static method or instantiates an object, client code calling your stuff has no choice in the matter. If I’m calling Foo’s GetTotal() method, it doesn’t make any difference to me if you call Utils.RunningSum() or just do the work yourself. It’s not as though I have any say in the matter. It’s not as though I can specify a strategy for computing the sum myself.

Now, by contrast, consider this:

public interface IComputeTotals
{
    int RunningSum(int n);
}

public class Foo
{
    public int GetTotal(int n, IComputeTotals computer)
    {
        return computer.RunningSum(n);
    }
}

Here I have a method that allows me to specify a number and a strategy for totals computing and it returns the computed total. Is this like inline? No, of course not — as the client, I have a lot of control here. Foo isn’t inlining anything because it needs me to specify the strategy.

But what about encapsulation? With the code in the method or abstracted to a hidden collaborator (static or new) the details of computation are hidden from me as the client whereas here they may not be (depending on where/how I got ahold of an instance that implements that interface). To that I say that it’s admittedly a tradeoff. This latter implementation is providing a seam and giving more options to client code whereas the former implementation is hiding more but leaving client code with fewer options. Assuming that any static methods involved are stateless/functional, that’s really the main consideration here – how much to cover up/hide and how much to expose.

I certainly have a preference for inversion of control and an aversion to static implementations both because of my desire for decoupled flexibility and my preference for TDD. But your mileage may vary. All I’d ask of anyone is to make informed decisions with eyes wide open. Metrics/smells like those about parameters and class/method size don’t exist in a vacuum. “Fixing” your 5000 line class by creating a static class and delegating 4800 lines of work to it behind the scenes is not reducing the size of that class in any meaningful sense, and it’s not addressing the bad smell the class creates; you’re just spraying perfume on it and hoping no one notices. The real problem isn’t simply that 5000 lines of code exist in one source file but rather that 5000 lines exist with no way to pry the dependencies apart and swap in alternate functionality.

So when you’re coding and bearing in mind certain heuristics and metrics, think of using static method calls and instantiating collaborators as what they really are — cosmetic ways to move code out of a method. The fact that the code moves to another location doesn’t automatically make it any more flexible, easier to maintain or less smelly. It’s providing additional seams and flexibility that has the effect you’re looking for.

By

Splitting Strings With Substrings

The String.Split() method in C# is probably something with which any C# developer is familiar.

string x = "Erik.Dietrich";
var tokens = x.Split('.');

Here, “tokens” will be an array of strings containing “Erik” and “Dietrich”. It’s not exactly earth shattering to tokenize a string in this fashion. And some incarnation or another of this predates .NET, C# and probably even my time on this planet.

It’s Actually Harder Than You’d Think to Split Strings Using Sub-Strings

But what about if we want to split over a string instead?

What about if we have “..” as a delimiter instead of ‘.’ and I want to split “Erik..Dietrich” in the same way? Probably an overload of String.Split() that takes a string instead of a char, right? Well, actually no. As it turns out, the API for string.Split() is pretty unintuitive.

First of all, that call to x.Split(‘.’) is not actually invoking Split(char), but rather Split(params char[]). (Notwisthanding the fact that this isn’t advertised in the MSDN page unless you drill into the individual method.)

So, calling x.split(‘.’) and x.Split(‘.’, ‘&’, ‘%’, ‘^’) are equally valid, syntax-wise in the case of “Erik.Dietrich” (and in this case, both will give me back my first and last name).

So, what one might expect is that there would be an overload Split(params[] string) to allow the same behavior as splitting over zero or more characters. Nope. Instead you have Split(string[] separator, StringSplitOptions options).

What’s Really Not Great about the Default Way to Split Strings with Sub-Strings

Two things suck about this.

  1. I have to specify some enum that I don’t care about in the first place and that has only two options, one of which is “none”. I mean, really? You can’t just assume “none” and let users specify a different case if they want with another overload?
  2. But what sucks even more about this is that params have to be the last argument in the parameter list, so that option is out the window. You no longer get that snazzy params syntax that the char version has, and now you have to actually awkwardly create a string array. So, here is the new syntax following the old. Note that the new syntax is pretty hideous.
string x = "Erik.Dietrich";
var tokens = x.Split('.');

string y = "Erik..Dietrich";
var newTokens = y.Split(new string[] { ".." }, StringSplitOptions.None)

This Gets a Lot Easier and Prettier using Regex.Split

I was getting ready to write something to hide this mess from myself as a client, when I stumbled across a better alternative than rolling my own extension method or string splitting class: Regex.Split(). Here’s how it works:

string x = "Erik..Dietrich"
var tokens = Regex.Split(x, "..");

No fuss, no muss, and exactly what String.Split() should do. Granted, the arguments to Regex.Split() are both single strings (so if you want to specify multiple delimiters, you’ll have to cook up a regex recipe) and it’s a static method, but it has the advantage of already existing in the framework and being a much, much cleaner API than x.Split().

Use in good health!

By the way, if you liked this post and you're new here, check out this page as a good place to start for more content that you might enjoy.

By

Unit Testing DateTime.Now Without Isolation

My friend Paul pointed something out to me the other day regarding my post about TDD even when you have to discard tests. I believe that this trick was taken from the book Working Effectively With Legacy Code by Michael Feathers (though I haven’t yet read this one, so I can’t be positive.

I was writing some TDD test surrounding the following production method:

public virtual void SeedWithYearsSince(DropDownList list, int year)
{
    for (int index = year; index <= DateTime.Now.Year; index++)
        list.Items.Add(new ListItem(index.ToString()));
}

and the problem I was having is that any tests that I write and check in will be good through the end of 2012 and essentially have an expiration date of Jan 1st, 2013.

What Paul pointed out is that I could refactor this to the following:

protected virtual int CurrentYear
{
    get
    {
        return DateTime.Now.Year;
    }
}

public virtual void SeedWithYearsSince(DropDownList list, int year)
{
    for (int index = year; index <= CurrentYear; index++)
          list.Items.Add(new ListItem(index.ToString()));
    
}

And, once I've done that, I can introduce the following class into my test class:

public class CalenderDropDownFillerExtension : CalendarDropdownFiller
{
    private int _currentYear;
    protected override int CurrentYear
    {
        get
        {
            return _currentYear;
        }
    }

    public CalenderDropDownFillerExtension(DateTimeFormatInfo formatInfo, int yearToUse) : base(formatInfo)
    {
        _currentYear = yearToUse;
    }
            
}

With all that in place, I can write a test that no longer expires:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Adds_Two_Items_When_Passed_2011()
{
    var filler = new CalenderDropDownFillerExtension(new DateTimeFormatInfo(), 2012);
    var list = new DropDownList();
    filler.SeedWithYearsSince(list, 2011);

    Assert.AreEqual(2, list.Items.Count);
}

In this test, I use the new class that requires me to specify the current year. It overrides the base class, which uses DateTime.Now in favor of the "current" year I've passed it, which has nothing to do with the non-deterministic quantity "Now". As a result, I can TDD 'til the cows come home and check everything in so that nobody accuses me of having a Canadian girlfriend. In other words, I get to have my cake and eat it too.

By

Regions Are a Code Smell

There’s a blogger named Iris Classon that writes a series of posts called “stupid questions”, where she essentially posts a discussion-fueling question once per day. Recently, I noticed one there called “Should I Use Regions in my Code?”, and the discussion that ensued seemed to be one of whether regions were helpful in organizing code or whether they constituted a code smell.

For those of you who are not C# developers, a C# region is a preprocessor directive that the Visual Studio IDE (and probably others) uses to provide entry points for collapsing and hiding code. Eclipse lets you collapse methods, and so does VS, but VS also gives you this way of pre-defining larger segments of code to collapse as well. Here is a before and after look:

(As an aside, if you use CodeRush, it makes the regions look much nicer when not collapsed — see image at the bottom of the post)

I have an odd position on this. I’ve gotten used to the practice because it’s simply been the coding standard on most projects that I’ve worked in the last few years, and so I find myself doing habitually even when working on my own stuff. But, I definitely think they’re a code smell. Actually, let me refine that. I think regions are more of a code deodorant or code cologne. When we get up in the morning, we shower and put on deodorant and maybe cologne/perfume before going about our daily business (most do, anyway). And not to be gross or cynical, but the reason that we do this is that we kind of expect that we’re going to naturally gravitate toward stinking throughout the day and we’re engaging in some preventative medicine.

This is how I view regions in C# code, in a sense. Making them a coding standard or best practice of sorts is like teaching your children (developers, in the metaphor) that not bathing is fine, so long as they religiously wear cologne. So, in the coding world, you’re saying to developers, “Put in your regions first so that I don’t have to look at your unwieldy methods, haphazard organization and gigantic classes once you inevitably write them.” You’re absolving them of the responsibility for keeping their code clean by providing and, in fact, mandating a way to make it look clean without being clean.

So how do I justify my hypocrisy on this subject of using them even while thinking that they tend to be problematic? Well, at the heart of it lies my striving for Clean Code, following SRP, small classes, and above all, TDD. When you practice TDD, it’s pretty hard to write bloated classes with lots of overloads, long methods and unnecessary state. TDD puts natural pressure on your code to stay lean, compact and focused in much the same way that regions remove that same pressure. It isn’t unusual for me to write classes and region them and to have the regions with their carriage returns before and after account for 20% of the lines of code in my class. To go back to the hygiene metaphor, I’m like someone who showers quite often and doesn’t sweat, but still wears deodorant and/or cologne. I’m engaging in a preventative measure that’s largely pointless but does no harm.

In the end, I have no interest in railing against regions. I don’t think that people who make messes and use regions to hide them are going to stop making messes if you tell them to stop using regions. I also don’t think using regions is inherently problematic; it can be nice to be able to hide whole conceptual groups of methods that don’t interest you for the moment when looking at a class. But I definitely think it bears mentioning that from a maintainability perspective, regions do not make your 800 or 8000 line classes any less awful and smelly than they would be a in language without regions.

By

Adding a Second Header to an ASP.NET Gridview

Last week, I had some specs to create a grid view in ASP that had normal column headers, as one might expect, and then also “super” column headers, where the columns were categorized. So, if, for instance, you had columns for “Dog”, “Cat”, and “Bird”, those three headers might have a “super-header” called “Pet”, in what basically amounts to a two-tiered categorization hierarchy.

Visually, what I wanted to do was create a second header row on top of the one populated from my data source, but there didn’t seem to be any good way to make this happen through the properties of the GridView. It also seemed like a mess to try to do this through CSS and somehow matching client side HTML styling with a service-side generated table. So, to combat this, I got ahold of the grid view’s table and manually added a row to it:

protected void GridView_DataBound(object sender, EventArgs e)
{
    var myGridView = (GridView)sender;
    if (myGridView.Controls.Count > 0)
        AddSuperHeader(myGridView);
}

private static void AddSuperHeader(GridView gridView)
{
    var myTable = (Table)gridView.Controls[0];
    var myNewRow = new GridViewRow(0, -1, DataControlRowType.Header, DataControlRowState.Normal);
    myNewRow.Cells.Add(MakeCell());
    myNewRow.Cells.Add(MakeCell("PETS", 3));
    myNewRow.Cells.Add(MakeCell("ZOO ANIMALS", 3));
    myNewRow.Cells.Add(MakeCell("VARMINTS", 3));
    myNewRow.Cells.Add(MakeCell("GAME ANIMALS", 3));
    myTable.Rows.AddAt(0, myNewRow);
}

private static TableHeaderCell MakeCell(string text = null, int span = 1)
{
    return new TableHeaderCell() { ColumnSpan = span, Text = text ?? string.Empty, CssClass = "table-header" };
}

What’s going on here is that when the grid is data bound, I check to see if data exists and, if so, I add my “super header”. This consists of grabbing the grid view’s first control, which is a table. This is kind of hackalicious in that a change to the ordering of the grid view’s controls collection is a breaking change, but c’est la vie. Once we’ve got ahold of that table, it becomes relatively straightforward to create a row for it using code and to assign it a class to allow further styling in CSS.

So, quick tip for today, and my first ASP.NET oriented one, as I learn the ins and outs of webforms and hopefully later ASP.NET MVC.

Cheers!

Edit: A reader named Patrick Hayden found this useful in his travels, but he was working in Visual Basic.  So for anyone interested in doing this in VB, here is his code:

Protected Sub GridViewBegroting_DataBound(ByVal sender As Object, ByVal e As EventArgs) Handles GridViewBegroting.DataBound
  Dim myGridView As GridView = sender
  If myGridView.Controls.Count > 0 Then
    AddSuperHeader(myGridView)
  End If
End Sub

Protected Sub AddSuperHeader(ByVal gridView As GridView)
  Dim myTable As Table = gridView.Controls(0)
  Dim myNewRow As GridViewRow = New GridViewRow(0, -1, DataControlRowType.Header, DataControlRowState.Normal)
  myNewRow.Cells.Add(MakeCell())
  myNewRow.Cells.Add(MakeCell("PETS", 3))
  myNewRow.Cells.Add(MakeCell("ZOO ANIMALS", 3))
  myNewRow.Cells.Add(MakeCell("VARMINTS", 3))
  myNewRow.Cells.Add(MakeCell("GAME ANIMALS", 3))
  myTable.Rows.AddAt(0, myNewRow)
End Sub

Protected Function MakeCell(Optional ByVal text As String = "", Optional ByVal span As Int32 = 1) As TableHeaderCell
' { ColumnSpan = span, Text = text ?? string.Empty, CssClass = "table-header" }
  Dim header As New TableHeaderCell()
  header.ColumnSpan = span
  header.Text = text
  header.CssClass = "table-header"
  Return (header)
End Function