Seven deadly sins of programming – Sin #2

July 24, 2006 at 5:33 pm

One of the guys on my wing during my freshman year of college was a trust-fund kid. His great-grandfather had done well in the market, and had left some money for each the great-grandkids.


Michael wasn’t paying for his schooling, nor was he paying for his cars, his clothes, his nice stereo, or his ski vacations. But that was okay with us, because he *was* paying for pizza, and he *was* paying for beer. Or his trust fund was.


Everything was fine until spring came around, and Michael got some bad news. His trust fund had been set up to carry him through 4 years of school, but because he spent money so fast, he had burned through all of it in less than two years. He was forced to get a job at the local grocery store to finish the year out, but couldn’t afford tuition and had to leave school and find a job to support himself.


========


It typically shows up in chapter two or three of the programming book. There’s a section titled something like, “What is an object?”, which speaks in flowing terms about the wonderful work of object-oriented development, and uses one of the following examples:



  • Geometric shapes

  • Animals

  • Musical instruments

  • Cephalopods (rare)

In this section, we find out that a square is an example of a polygon, a cheetah is a cat and also a mammal, and so on.


All of this to introduce is to the concept of “is-a”.


We then see an example where we can ask any polygon how much area it covers, tell any mammal to walk, and tell any cat to ignore us while we’re talking to it, all through the wonders of inheritance and virtual methods.


Good taste and a firm grasp of the relative usefullness of this concept would dictate spending 10 or 20 pages explaining these concepts, but most texts significantly exceed that, and most programming assignments spend a fair bit of time on that as well. Kindof like how linked lists rule your life for a month or so.


It’s therefore not surprising that many younger developers think that inheritance is a feature that you should, like, “use” when you write software.


There are a few problems with this.


The first is that “is-a” is, in my experience, a pretty rare relationship between objects. More common is the “looks the same on one dimension but has different behavior across another dimension”.


Good luck implementing this:


public class Monotreme: Mammal
{
    public override GiveBirth()
    {
        // add appropriate implementation here
    }
}


That’s the sort of thing that tends to be non-obvious when you first create an object, but annoying obvious when you’ve bought into the whole inheritance mindset.


That’s not to say that inheritance isn’t useful. It’s just to say that you should understand that a MemoryStream isn’t really a Stream, at least in the true “is-a” sense, and be prepared to deal with it.


The second problem is more philosophical and aesthetic. I recently wrote code like this:


class ClimbList: List<Climb>
{

}


Which seems like a perfectly reasonable thing to do, at least on the surface.


But the surface – or at least the surface area – is a problem. List<T> is a pretty extensive class, and when I defined ClimbList, I was saying that ClimbList does the proper thing when any of the List<T> methods or properties are called.


I’m pretty sure that’s not true. Or at least, I’m not at all sure that it *is* true (I have no tests to support such a belief), but users of ClimbList don’t have any way of knowing that the only methods I’m currently using are Add() and the indexer. Intellisense brings up all of them when I go to call one of the methods I wrote.


Which brings us in a long and roundabout way to our penultimate sin:

Sin #2 – Overuse of Inheritance

So, what should you use if you don’t use inheritance (and I am advocating that you approach it carefully and thoughtfully – it’s a similar decision to adding a virtual method)?


Composition. Make the object a field inside of your object, and then write the forwarders that you need (it’s usually not more than one or two).


And, if you really need inheritance in your design, add it carefully and thoughtfully.

Seven Deadly Sins of Programming – #3

July 18, 2006 at 5:19 pm

I’m sure this is going to be a contentious one. I don’t have a good story about this one, so I’m just going to go right to the sin.


Sin #3 – Overuse of Virtual


There are two schools of thought when it comes to virtual. They are:



  1. Always use virtual

  2. Only use virtual when absolutely necessary

The first group’s argument is that original designer of a class is a poor judge of how the class might ultimately be used. Making all methods virtual allows the users of the class to easily extend/modify the behavior. Non-virtual methods make more work for the user.


(or that’s the argument I’ve heard – please comment if there’s something I missed…)


The argument is true. Virtual does make extensibility easier. The problem with virtual is robustness.


The original designer of a class has a good idea for what the class is about, and the implementation is designed (either explicitly through the tests written with TDD, or more implicitly through the mind of the designer) to support that idea (or contract, if you prefer).


In other words, the designer has an explicit idea about what extension points he wants to support in the class. If virtual is only on those points, then the designer has (or should have) done sufficient testing on those points, and it’s pretty likely that users who extend through those points get the behavior they want. And since only a few methods are virtual, the fact that a specific method is virtual is an important clue to the user that that method *is* an expected extension point.


If virtual is on every method, the user can extend in a lot of ways, but it’s very unlikely that the designer thought of all those ways – or all combinations of those ways – which means that such a user extension is going into uncharted territory. Maybe it doesn’t work when you try it. Maybe it works now, but breaks with the first update when the object behavior changes.


I prefer the “you can only do these 2 things, but I promise that they work” classes to the “you can do any of these 12 things, but they may or may not work” classes.

Seven Deadly Sins of Programming – #4

June 26, 2006 at 5:38 pm

Our next sin is the one that I’ve certainly been prone to.


Long ago my wife and I owned a house east of Renton, Washington (those who know about Renton at that time can probably understand why one would say “east of Renton”).


Like many homes, this one had had various indignities committed on it by the previous owners. One of the most obvious was what can only be described as a “lean-to”, constructed out of surplus Boeing shipping containers and shielding the washer and dryer from the rest of the garage.


After dealing with cold and dirty feet for a few months, we decided to convert the space to a real room, with features such as a floor, a laundry sink, a ceiling that was actually attached to the joists, etc. The framing went quickly, but when it came to the point of finishing the drywall, we ran into a problem.


My wife and I had very different ideas of how smooth a wall should be before it was painted. She wanted it to be mostly smooth, while I spent a fair amount of time with a light at an oblique angle, trying to get it really smooth.


I did the bulk of the drywall finishing myself in that room.


And after it was all painted, it turned out that it didn’t really matter. Her standard was fine.


Which leads us, in the usual roundabout way, to our destination:


Sin #4 – Premature Optimization


Or, in other words, spending time on things that don’t really matter in the end.


It’s happened to all of us. You’re in the middle of writing a class, and you think “a linear search of an array just isn’t going to be fast enough here”. So, you spend some extra time, and you use a hash table, or a tree, or a binary search, or something like that, and when you’re done, you say to yourself, “now *that* isn’t going to cause any problems”.


And then later, when you’re getting close to done, you run your code through a profiler, and you find that a) the array never has more than 3 elements and b) the code is spending a ton of time elsewhere in the code.


You wasted the time that it took you to do the optimization (bad), and you may have ended up with code that is considerably harder to read (worse), depending on how much optimizing you did.


How do you avoid this? I have two bits of advice:



  1. YAGNI came out of XP, and it’s a good dictum to follow. If you are doing TDD, this is easier to do as you are more focused on making one small change and not on “finishing the class”.

  2. Go read what a wise man has said about performance, and work on creating a performance culture in your group.

A close escape, and more on inappropriate cleverness

June 2, 2006 at 11:53 am

Monday I went on the 7 hills ride.

Wednesday night, I wrote a long and detailed post about it, and hit the “Post” button.

In a fit of editorial brilliance, my blog server chose that exact moment to experience a problem, and the post vanished into the ether.

Few will quarrel that that event was a net positive for western civilization as a whole.

So, here’s the short version.

HomeDownPauseUpDownUpDownUpDownUpDownUpDownPauseUpDownUpDownStrawberryShortcakeUpHome

Now, that that’s out of the way, there were a few comments on the 6th deadly sin post that I’d like to comment on:

(BobHy) “Inappropriate” is relative to time (as your first story illustrates) and point-of-view (“cui bono”).  Would you agree that cleverness which benefits the user who runs the code (efficiency) or the team who maintains it (maintainability) is appropriate?  And if the clever bits trade off one of these against the other, who gets to decide the direction?  

Inappropriate is obviously a value judgement. Who decides? Well, I do, since it was my post. In general, I’d expect that the team would decide, though in my experience there aren’t that many teams with the sort of culture that allows this.

But, yeah, I agree with this.

(BobHy) OTOH, even when we consider cleverness whose only benefit is to amuse its author, it’s fair to ask whether the author’s happiness is itself a benefit to the user community (“a happy programmer is a cooperative, bug-fixing programmer”) or to the team (“a happy programmer remains on the team and trains his/her successor”).  

An ancient Chinese programmer once said,

I pity the fool who has to maintain or extend the code of a programmer who wrote clever code because it made him happy.

I’ve experienced many cases where a developer writes code that is clever, moves on to another job, and then the people who have to deal with the code have to deal with that cleverness. My current team is dealing with this now, and it’s a big problem for us.

If you want to amuse yourself writing clever code, do it on your own time. But don’t make your team deal with it.

(BobHy) I’m beginning to think “inappropriate cleverness” may be a vacuous term. But there certainly is “incorrect” cleverness, where the claimed benefit of the design is refutable.

You are obviously not a long-term reader (assuming, for the sake of argument, that such a person exists) if you find the presence of a vacuous term in a post of *mine* surprising.

The difference between inappropriate and incorrect seems mostly one of semantics. I chose inappropriate because I think it’s more of a judgement call, while incorrect implies a black and white choice.

(Tomer) It’s funny, I keep running into arguments as to the pros and cons of the sort of “codespace-optimized” loops you mentioned. I’m actually very much for them – they’re usually used where the algorithm is very simple (for example, when writing to a frame buffer and adding a stride value to your target pointer) and tend to make the code “look” a lot more elegant.

I will confess to being somewhat of a code aesthete in my earlier days, looking for more compact expressions. I got started in that back when I was writing games in interpreted basic, and the size of your code could have a big impact on how fast your program ran.

Over the years, writing a lot of code and trying to understand other people’s code, I realized that the #1 most important feature of well-written code is clarity. And for me, the two most important facets of that are:

  1. Predictability (the code in method a looks like the code in method b)
  2. Simplicity

The world where for loops just do looping is a conceptual simplification over the world where they sometimes do additional things. In the first case, I read the for statement once, and then concentrate on the loop body. In the second case, I read the for statement, read the loop body, but have to remember that there’s something else going on that I have to go back and refer to later. When you write that, you’re making me work harder, when you simply could have put “j += stride;” as the last statement in the loop.

That’s why foreach is such a useful construct. It’s not that you type less. It’s that it’s conceptually simple – you know that there’s no funny business going on, and all you have to worry about is the loop body.

(Shahar) We used to have a dev in a previous company I worked at (circa 2000) who had his own special way of doing HRESULT-COM error checking..

Shahar goes on to detail what the practice was, which I agree is bad, but it’s a matter of degree, since all HRESULT handling techniques suck.

*****

Most code is read far more times that it is written. Go read this post by Peter Hallam, one of the C# compiler devs.

Seven Deadly Sins of Programming – #6

May 31, 2006 at 11:14 am

Seven Deadly Sins of Programming – #6


Way back in my early career, I worked on a graphics program written in FORTRAN that was used to plot engineering data.


It ran on Tektronix 4014 terminals.


Our application – known as E.G.G. (Engineering Graphics Generator, one of a whole series of not-very-creative punny names used by the group) – used a menu-based approach. The user would place the cursor (moved by the thumbwheels on the right) over a menu option, hit the spacebar, and then the code would figure out what to do.


The internal architecture of the app was really pretty good. There were, if I recall correctly, 39 different screens, and each item on the screen had an id associated with it. The code would find the right item on the page, come up with a numerical code (menu number * 100 + item number), find that in an array of function pointers, and then call the appropriate function.


It worked fine, and allowed us to have a batch mode as well.


But after I started playing around with it a bit, I noticed that the command dispatch was a bit slow. After you selected an item on a screen, it took at least a couple of seconds for the screen to erase and redraw (this was running on shared Vaxen, so you didn’t get much CPU time, and what you got was fairly slow).


I did some digging, and found that the original developer (no longer with the group) had written the item lookup code using a Fibonaccian search.  This is a search that is similar to binary search, but doesn’t require complex operations such as divide by two or shift right (that alone should tell you the vintage of this search).


Such a search *may* have made sense on earlier systems, but on Vaxes, both the divide by two or shift right are in the hardware. So, we were using a search method optimized for constraints that we didn’t have that nobody in the group understood, in an attempt to be faster than binary search.


But it was a *clever* algorithm.


At this time, we had about 1200 items in our dispatch table. The first thing I did was build sub-index that stored the starting index of the first item in each menu, which meant the search space for a typical menu item went from 1200 items to about 30 items. The second thing I did was switch to interpolation search, which, given the nearly perfect distribution of items numbers within a menu (most menus had item numbers from 1-n, with very few holes), gave an almost perfect search. And it was code that most people in the group could understand.


So, the dispatch time went from a few seconds to about half a second, and I wrote up a “cost savings” in which I multiplied the number of users we had by the amount of time they used the application by the number of menu picks per hour, and figured out that I had saved the company around $30K per year. For which I got a $100 gift certificate.


So, what’s the point of that story?


Well, it makes me look good, which is the primary goal. It also illustrates that cleverness is an over-rated attribute of developers. How many times have you seen:


for (int i = 0, j =bounds; i < upper; i++, j += JUMP_SIZE)
{
    …
}


That’s somebody who is trying to be clever. They should have written:


int j = bounds;
for (int i = 0; i < upper; i++)
{
    …
    j += JUMP_SIZE
}


Which takes us on a long and tortuous trip to


Sin #6 – Inappropriately clever code


What examples of this have you seen?

Seven Deadly Sin of Programing – #7 – Excessive coupling

May 23, 2006 at 11:43 am

(these will be in reverse order…)


#7 – Excessive Coupling


We start out with a simple one.


Coupling is obviously a necessary evil. The Console class, for example, wouldn’t be able to send text through Win32 without being coupled to Win32.


Given that, less coupling is nearly always better that more coupling. A few cases that I see:



  1. Using switch statements rather than polymorphism or interfaces.

  2. Classes that do too much, rather than being focused on a single goal.

What do you think? Do you have more examples?

Random sometimes, not random other times

May 19, 2006 at 11:43 am

From a email:


private void button1_Click(object sender, EventArgs e)
{
   Customer c = new Customer();
   c.Randomize();


   Customer b = new Customer();
   b.Randomize();


   MessageBox.Show(string.Format(
        “object c random number = {0}, object b random number = {1}”,
        c.RandomNumber, b.RandomNumber));
}


public class Customer
{
   private int random = 0;


   public void Randomize()
   {
      Random r = new Random();
      random = r.Next();
   }


   public int RandomNumber
   {
      get { return random; }
   }
}


If I run the above code without debugging it always returns the same random number, but if I step through the code I get different random numbers.


******


This is a variant of a question that comes up fairly often.


When you create an instance of the Random class, the pseudo-random number generator needs to be seeded with an initial value. Some generators always seed with zero, which means you always get the same sequence of numbers. The .NET Random class seeds using the time, which means you get different results from run to run without having to set the seed yourself.


It also means, however, that if you create two Random instances one right after another, the seed value (I think it uses GetTickCount()) hasn’t had time to change, and you get the same sequence. But if you run it in the debugger, enough time passes between the creation of the two instances that you get different seeds and different numbers.


I think the best solution here is to move the Random instance to a static field:


static Random r = new Random();


And then just use that instance of Random from all the Customer instances.

Worst… Code… Ever…

March 16, 2006 at 12:24 pm

Like many developers, we enjoy making fun of other people’s code. I’ve had lots of those discussions over the years. And when it comes to who has had to work with the worst code, I’ve never lost.


Way back when I was just out of college – when 7-bit ASCII ruled the world, and people hadn’t decided whether lowercase characters were a fad or not – I worked for a Large Seattle Aerospace Company, at Large Seattle Aerospace Company Computer Services, in a group that built graphics tools for engineers.


One of the libraries that we owned was generated code.


Now, I don’t hate generated code. The Windows Forms Designer and I have a generally felicitious relationship. I even have a patent cube (which you get when an application is accepted at Microsoft) sitting on my windowsill that is related to an architecture for doing generated code.


This particular library was special. It goes without saying that the source to the generator was lost in the card recycling bin of time, but that was not what made it unique.


First of all, it was in FORTRAN. And not that namby-pamby FORTRAN where you can have modern control structures, this was FORTRAN 77, and you had better know how to count spaces and what it meant to put a character in column 6. Did you think that python came up with the idea of significant whitespace? Pshaw.


Secondly, the programmer who had written the code generator was a bit of a FORTRAN afficianado. There’s a feature in FORTRAN 77 known as the “assigned goto”. Here’s an example:


      ASSIGN 30 TO LABEL
      NUM = 40
      GO TO LABEL
      NUM = 50                ! This statement is not executed
30    ASSIGN 1000 TO IFMT
      PRINT IFMT, NUM         ! IFMT is the format specifier
1000  FORMAT(1X,I4)
      END


Now, to understand this, you have to remember that by default, any variable that starts with the letters I-N is implicitly an INTEGER variable (all others are implicitly REAL). So, you can assign 30 to LABEL (this is *not* the same thing as writing LABEL=30, which means what you expect it to mean), and then use “GO TO LABEL” to goto 30.


Suffice it to say that the developer had never read Dijkstra.


Now, my guess is that while the vast majority of you are thankful that you don’t have to work in FORTRAN, there is a divide in opinion beyond that. Some of you are saying “Well, that’s not really that bad”. But the rest of you are shaking your heads, because you know what is coming.


When you’re doing code generation, you need to somehow come up with variable names. In this case, the developer took the easy way out, and all integer variables were something like “I1344”.


And line numbers also use the same range, starting at 1000 and going on up.


So, it means that the code has lots statements like:


GO TO 11634


and lots statements like:


GO TO I1655


Did I mention that in the fonts of the day, the difference between “1” and “I” was fairly subtle? Even if you did notice the I in front, you had to cope with the fact that


GO TO I1655


really meant


GO TO 3455


At least, it meant that sometimes, but I1655 would be re-assigned when the program ran.


IIRC, there were about 15K lines of this in library.


So, bother me not with tales of poorly-formatted C# code or 2000 line C functions. They are mere trifles. To snatch the rock from my palm will require something stronger. Are you up to the challenge?


(I am a bit worried that there might be some LISP or APL programmers out there…)

TDD and design methodologies

March 7, 2006 at 1:24 pm

(something I posted on our internal agile alias, in response to a question about how design works in TDD…)


There’s an underlying assumption in software engineering that “more design == better design”, despite that fact that the vast majority of us have worked with baroque systems that answer a bunch of questions that nobody ever asked.

 

The traditional theory is that if you don’t do the up front design, your code will be poorly architected, inflexible, and you’ll be in trouble when you try to maintain it. Which is true. But it’s also true that up-front design – especially the “spend a milestone” type of up-front design – often leads to the same result.

 

The ideal architecture is a minimalist one. It provides all the features that are needed and no features that aren’t needed (I mean “features” in the class method sense, not the user-visible sense)

 

The up-front approach attempts to do that without the data around which features are needed and which ones aren’t needed, which always changes along the way.

 

TDD says, “We’re going to figure out what we need and how to put it together along the way. We know we’re not going to get it exactly right the first time, but with our tests we can refactor as necessary

 

Comments?

Practical Tips For Boosting The Performance Of Windows Forms Apps

March 7, 2006 at 12:18 pm

Practical Tips For Boosting The Performance Of Windows Forms Apps