The Daily Parker

Politics, Weather, Photography, and the Dog

Code smells

A code smell happens when a piece of software code looks like there might be something wrong with it, but you can't quite tell what. You use the smell to figure out where the bad code is hiding. Martin Fowler has devotes an entire chapter of Refactoring to code smells.

Here is an example, from a class that returns configuration information:

public string Read() { ... }

public double ReadD() { ... }

public int ReadN() { ... }

public string ReadString() { ... }

What's wrong with this code? Several things:

  1. It does the same thing four times.
  2. Having two similar methods that appear to return exactly the same data raises a red flag, because you don't know from looking at them why they differ but you feel like they differ for a reason.
  3. Despite the code's appearance, it actually offers no guarantee that the data requested will be of the correct type, only that the data will have a particular type.
  4. if some random piece of code requires a data type it doesn't support, you'll have to add yet another method to the class, or change the returned data to suit what you need, neither of which makes a lot of sense.

This code came from an ancient, functional-procedural outlook in which the type of data returned actually matters. Object-oriented design cares more about what the data represents. In other words, this code is like multiple different filler tubes on your car, one for each grade of gasoline.

There's more. A little farther on, we find these methods:

public int ReadArray(string, ref String[]) { ... }

public int ReadArray(string, ref int[]) { ... }

public int ReadIntArray(string, ref ArrayList) { ... }
	

In all, there are 14 "Readtype" methods on this class, some that return the data sought, others that return a status indication and pass the data back through a by ref parameter.

And, of course, since this is in a fundamental class, none of these methods has fewer than 5 references to it in the application, and some (like ReadN) have 34, making refactoring unusually difficult. I'll post again when I figure out how I'm going to do that.

Coat-check claim number

Yesterday at the gym's coat check, I got claim check 404.

The coat-check guy didn't find this nearly as funny as I did.

Even more funny—to a software nerd, anyway—was that when I gave him check #404 after working out, he found my coat.

One can imagine other possibilities. But 404 is the best, I think.

Compiler warnings

Read, understand, and then fix your compiler warnings.

Compiler warnings let you know that you've either done something wrong, or you've done something non-standard. Either way, ignorning compiler warnings shows a lack of discipline and skill; it's something like ignoring big red "warning" signs in real life.

I'm working on a .NET solution that, when last compiled, generated over 60 warning messages. A couple of them I put in to let other developers know about problems I found, but most warned about things that actually needed to get fixed.

For example, the following line of code:

if (comboBox.SelectedItem == "Do stuff")

generated the warning, "Possible unintended reference comparison; to get a value comparison, cast the left hand side to type 'string.'"

In other words, the if statement was evaluating whether the object comboBox.SelectedItem was the same object as the string "Do Stuff," which is an impossibility. So the comparison would always fail, making it look like the feature was failing. Yet the compiler warned the developer about the problem, and even said how to fix it.

If you're wondering, the corrected line looks like this:

if ((string)comboBox.SelectedItem == "Do stuff")

Source-control etiquette

<Rant>

Any software project that has more than one developer working on it needs to have some way of ensuring that there is one and only one "official" version of the code. This is called source control, for which teams use tools like Microsoft SourceSafe and Rational ClearCase.

In the land of myth and legend, the code checked in to source control is ready to roll. Checking something in that doesn't work, or that prevents other parts of the software project from working, is called "breaking the build." On some teams breaking the build results in the offending developer working late, suffering humiliation from his peers, or having Vinny come by and break his knuckles.

Adhering to this discipline allows developers to join the team, get the latest copy of the code, and start working on it. Failing to adhere to this disicipline causes anguish, frustration, and despair.

That is all.

</Rant>

Also on my reading stack

I just finished Garbage Land, leaving only about a dozen books on my reading stack right now. Highlights:

Why is this in the Software category? Because better wetware means better software.

It's important to read widely in order to write better, whether your language is English or C#. Read as much as you can, about anything that interests you. Limit your professional reading to 50% of your total no matter what (but shoot for 25%). The more you know about things outside your profession, the more you can bring to the profession, whether it's software or anything else.

Comments working

Aha.

In ASP.NET 1.1, you need to have a folder called aspnet_client\system_web\{.NET version} under a Web application's root in order for Javascript to work.

In ASP.NET 2.0, you don't. And in fact, on a server (like mine) where both versions are running side by side, having that folder causes Javascript to fail in some browsers on the ASP.NET 1.1 sites (like this one).

This means comments are working now.

But I'm still going to install Community Server, though I probably will keep Das Blog now. (For the record, I always thought it was a configuration error, not Das Blog's fault.)