Copy-and-paste coding

Jul 4, 2007

Copy-and-paste coding drives me crazy. Here’s some Javascript that I was passed today that illustrates the problem:

if (navigator.userAgent.toLowerCase().indexOf("safari") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
} else if (navigator.userAgent.toLowerCase().indexOf("opera") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
} else if (navigator.userAgent.toLowerCase().indexOf("firefox") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
} else if (navigator.userAgent.toLowerCase().indexOf("netscape") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
} else if (navigator.userAgent.toLowerCase().indexOf("msie") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
} 

The person who wrote this probably started with:

if (navigator.userAgent.toLowerCase().indexOf("firefox") >= 0) {
  if (pageSize > 1000000) {
    hideLink();
  }
}

then decided to expand the code to test other browsers as well. So what did they do? They copied the lines and pasted them, edited a bit, pasted again, edited a bit, and so on, until they had code to cover all the situations, ending up with 21 lines of code: 15 are exact copies and 5 are very similar to each other. Ho boy.

This is an example of copy-and-paste coding, and it has three disadvantages:

  • it bloats your code, and as we all know the best code is no code at all

  • it makes your code less efficient, because you repeatedly do the same thing; for example, the above code converts the user agent string to lower case every time it hits one of the main conditions

  • it makes your code much harder to maintain: imagine you have to maintain this code and work out that actually the test navigator.userAgent.toLowerCase().indexOf(...) isn’t the right test. Because the code is simply repeated, you have to edit five lines. What about if the numeric limit of 1000000 needs to be changed? You have to edit five lines. Add an argument to the function call? Five lines.

Copy-and-paste once, and edit heavily, by all means, but if you find yourself copy-and-pasting code repeatedly, with small edits each time, Stop. Think. Refactor. Extract the commonalities, with the aim of coding each thing just once (not just the processor doing each thing once, actually writing each thing once). Think about the changes that someone might need to make to your code, and factor those parts out so that they’re easy to change.

OK, so I guess I have to put my neck on the line and give an example of how I’d write it. Assuming that you do actually need to have individual control over the allowed page sizes for each browser, then

var browser = navigator.userAgent.toLowerCase();
var defaultLimit = 1000000;

var limits = new Array();
limits["safari"]   = defaultLimit;
limits["opera"]    = defaultLimit;
limits["firefox"]  = defaultLimit;
limits["netscape"] = defaultLimit;
limits["msie"]     = defaultLimit;

for (limit in limits) {
  if (browser.indexOf(limit) >= 0 &&
      pageSize > limits[limit]) {
    hideLink();
    break;
  }
}

does the trick. I’m getting the lower case browser name only once. The tests aren’t repeated, so if they change, I only have to edit in one place. The most likely things to change are split off into variables at the top of the code, so that they’re easy to locate and easy to alter.

Why do I even bother to write about this, something that surely every coder knows? Because I’ve seen programmers, real life ones, ones who earn money as developers, doing copy-and-paste coding, and I’m fed up of being the muggins who has to wade through it, copying and pasting the same fixes, or rewriting swathes of it that they couldn’t be bothered to refactor themselves along the way. I’m tired of picking my way through code to confirm that, yes, this really is a character-by-character verbatim copy of exactly the same very complex XPath used just three lines above. And don’t get me started on the same code repeated in separate modules. Argh.

Show some empathy for the person who’s going to be maintaining your code. Who knows: it might even be you.

The perils of default namespaces

Jul 1, 2007

A lot of people run into problems with namespaces, and most of those arise from using default namespaces (ie not giving namespaces prefixes). The transformation technology you use can have a big effect on how confusing and irritating it gets.

Default namespaces make XML documents easier to read because they allow you to just give the local name of an element rather than using prefixes all over the place. For example, using:

<house status="For Sale" xmlns="http://www.example.com/ns/house">
  <askingPrice>...</askingPrice>
  <address>...</address>
  <layout>...</layout>
</house>

reCAPTCHA on comments

Jun 30, 2007

I’ve installed reCAPTCHA Drupal module as a way of combating the large amount of comment spam that I get on this blog, and to support the reCAPTCHA project. What a fantastic idea! I see that there’s already a term for using human expertise in a fun or transparent way to solve problems that can’t be solved by computers: bionic software.

ReCAPTCHAs are fun and add to human knowledge, but if you want to avoid them (they do take a little time to load) then create yourself a login. On the other hand, if you have some spare cycles, then you can do reCAPTCHAs on the reCAPTCHA site without creating lots of comments here.

Do we need more women in computing?

Jun 21, 2007

Woah, so Tim took me seriously about linking to women’s blog posts from his own, and suddenly I get readership! Edd phrases what I was trying to say better than I did myself:

As Jeni brings her article to a close, it’s with some shock and shame that I get the punchline loud and clear: “this isn’t about you.”

It’s about empathy, inclusivity and selflessness. Human qualities that are unrestricted to either gender.

But I wanted to address some of the comments that question whether we should really care about this. For example:

Why we need to have an equal number of men and women in every job field known to man is beyond me. Women freely choose not to be programmers. That is manifestly not a problem, and certainly not a problem that needs fixing. I try to respect peoples decisions about what they do with their lives, so I for one vow to do absolutly [sic] nothing about this.

Lessons learnt

Jun 19, 2007

I’m coming up to completion on the project that I’ve been working on for the last six months or so. It’s been very different from the projects that I’m used to: usually I fly in and either write a stylesheet or schema to a given specification and hand it off, or have to wade through, critique and improve a lot of existing XSLT code or schemas. Here, I’ve been involved in a much more end-to-end way: having to do the technical specification myself, do a lot of the testing, and deal with the bugs. Plus half of the project has involved customising existing (complex) stylesheets, written by someone else.

So, what have I learned?