How to commit code

When you commit (check in) your code to the source control system there are some things you should do to ensure quality and trackability.

1) If your source control system supports change sets, that is commiting a set of files as a bundle, make sure you just include one issue in that bunch. You shouldn't fix a bunch of things in all ends of the project and then commit them all in the same change set. The change set may well span over multiple projects tho, because one change set should include all the changes done to resolve that particular issue. A change set should also be compilable upon commit and not rely on the next one to be able to work.

2) Before you commit your change set you should diff every file against the repository version and see that you only commit things that were intended to commit. It's pretty easy to commit code you commented out, temporary variable names or debugging code if you don't review your own commits. If your commit is in central parts of the application or very large it is good to have another team member sit next to you when reviewing the changes.

3) When you've limited your change set to include only one issue and reviewed all the changes done, you should write a short description of the content in your change set. This is written as the "commit comment" and will be visible when you look at the log for your repository. Since your change set only should deal with one issue it is easy to write a brief description of what you've done. It's also good to include an issue id if you have an issue tracking system.

I guess many readers might think that this takes a lot of time - but think of all the time you save due to the higher quality instead! I've done this with all commits for many years now and it is very uncommon that I add bad code to the repository. It's not at all uncommon that I notice bad code while doing my personal code review upon commit tho!

My biggest problem when converting to this more professional approach was to limit my changes to only one issue. Yet today there are times when I can't commit only one change as I've fixed two (or more) issues in parallell without commiting the first one - and when they touch the same file it isn't possible to have only one change in the change set. If one of the fixes are small I usually solve this by reverting the changes for that issue temporarily while commiting the first change and then redo the changes for the next commit - but once in a while I need to write a commit comment with the dreaded word "... and ...".

I love deleting code

Yesterday I read a tweet saying "the next best thing after writing code is deletig code". My response was fast, saying "personally i like deleting code more. ;)". Let me elaborate on that!

Deleting code means that you either:

• found a better way to do something
• found unneeded abstractions
  
• found unneeded functionality

So, deleting code (at least when it's done on purpose ;)) is always done because you don't need it. Taking away code that you don't need is great, because the less code you have, the less can go wrong, and after the code is removed there is less to test and less to maintain.

Therefore, always strive to have as little code as possible doing the job. As they say in Extreme Programming, "Pay as you go: Build just enough to meet today's requirements".

You could get sad when removing code because it means that you or someone else have done something that could be considered a waste. Well, sometimes it was a waste but it won't be less a waste because you keep it. Most times the code you're about to delete served a purpose though, leading you to find the better solution.

So summing it up; Don't be sad about deleting code, love it like I do!

Odd way to fill your mailbox

On a site of mine I get an email every time an invalid url is requested. The purpose of that is mainly to find broken links. Sometimes a misconfigured crawler may spam me with a hundred mails, but they're pretty easy to delete and it doesn't happen frequently.

Tonight however, from 21.08 to 21.53, I received more than 3.500 such, all from the same ip. It would probably have been more if i didn't block that ip at 21.53, as I luckily was at the computer. The requested url:s were all directories and pages that exist on the site, but combined in odd ways, primarily stacking directories in long chains that doesn't exist.

As I don't have access to the firewalls of my hosting company I had to figure out a way to block the weirdo myself. My solution was to just terminate requests from that host in my asp.net page like this (but the real ip instead of the x:es):

  protected void Page_Load(object sender, EventArgs e)
  {
      if (Request.UserHostAddress == "x.x.x.x") {
          Response.End();
          return;
      }
      ...
  }

The Anti-IF Campaign

When I first found Francesco Cirillo's Anti-IF Campaign I signed up almost instantly. Over the years I've learned that the if-statement is best used sparsely. Neither the campaign, nor I, strives to eliminate all if-statements but rather wants you to think twice before using them. Unfortunately I don't think the web site does a good job explaining why if is bad, so I'm going to make a try on my own.

Every if-statement creates another path through the code. That opens up for a lot of additional cases to test and hence harder to test. Harder to test means there will likely be more bugs. For example you need to add at least one Unit test for each if you add to your code. A nice technique for testing your code coverage is to comment out either the if or the content and see if any test fails. If it doesn’t the code is either unnecessary or not covered by your tests.

That being said code is a lot about different paths, and must be, so you can’t take away all if-statements. But consider your options!

There’s the redundant if:

if (flag) {
 flag = false;
}

Just replace that with

flag = false;

There’s the horrible null check:

if (filter != null) {
 filteredList = filter.Filter(list);
}
else {
 filteredList = list;
}

Create a null object filter and always filter, like this.

• If don’t have a interface for your filter already, create one.

interface IFilter {
 Array Filter(Array list);
}

• Make a implementation of that interface that just returns the argument.

class NullFilter : IFilter {
 Array Filter(Array list) {
   return list;
 }
}

• Where you used to decide not to set the filter field, create a NullFilter.

IFilter filter = new NullFilter();

• Now remove the if and the else and just filter.

filteredList = filter.Filter(list);

Now lets round up with the case that Francesco lists on the Campaign site where you refactor your code to use strategy objects.

The Simplest Anti-IF Code (Anti-IF Campaign)

There's a lot of other cases, but I can't go through them all. I hope that you've learned that if should be avoided and that you will try to do so in the future.

I'll end this article with a Twitter quote from @garybernhardt.

Let's rename the "if" construct to "ponder" and impose a one-second busy wait per use. That'll teach those branchers! ;)

SRP subsumes ISP

Yesterday I listened to a friend who held a presentation about the SOLID principles. You know, the five principles in object oriented design that Uncle Bob put together.

Single Responsibility Principle
Open/Closed Principle
Liskov Substitution Principle
Interface Segregation Principle
Dependency Inversion Principle

If you're not familiar with them you should read up right now. Here's a good place to start.

Anyway, when my friend reached the I of SOLID it struck me that ISP is only interesting if you've violated SRP to start with. I've always thought of it as a refactoring pattern, but all the definition says is that "make fine grained interfaces that are client specific". Adhering to the SRP it would be difficult to make interfaces any other way... I decided to confront Bob with this, and he seems to agree:

I asked:
would you agree that ISP is self-fulfilled if SRP is used? ie, ISP only has a value on its own when working with bad design?

And he replied:
Strictly speaking, SRP subsumes ISP. The problem is you can't always be as strict about SRP as you'd like.

So, ISP is mainly an option if you fail to design by the most important principle. But as Bob states, it's not at all uncommon that you do that so there's still value in the principle.

Also, I think ISP is a great principle when dealing with legacy code. If you have a huge class it may be both difficult and very time consuming to refactor it to meet SRP. Here I use ISP as a lightweight SRP, creating the interfaces that I'd like the objects to be in a good SRP design and then have the same class implement them all. It takes virtually no time, and then I can start writing good code from that point on. It also helps me if I want to refactor out one of the many parts this legacy class may contain.

If I'd end up with some kind of conslusion it would be that you shouldn't think much about ISP when designing new software, but it's a great principle for refactoring!

Learning XP

I'm currently having a great time at work, developing a web application from scratch using Extreme Programming. You can follow our stumbling steps in a new blog, XP Creuna.