Skip to content

Spolsky and CHG are Idiots

April 6, 2009

Joel Spolsky and Coding-Horror-Guy (aka Jeff Atwood) are idiots.

Ok, well, maybe they’re not complete idiots, and, in fairness, everyone is stupid and lazy to some degree or another (myself most definitely included). The thing about Joel and CHG is that, in general, I have a lot of time for them (some might say too much), but sometimes they seem to grasp completely the wrong end of the programming stick.

Take, for example, this post, which is a transcript from one of their podcasts. In it, Joel and CHG discuss “SOLID Principles“. For a start, Joel says “It’s object-oriented design, and they’re calling it agile design, which it really, really isn’t” – but the title of that page is “The Principles of OOD”. Doesn’t he realise what OOD stands for? Or maybe he’s not referring to the article, but the podcast; but that’s entitled “…the SOLID Principles of Object Oriented Design”.

And this isn’t even the reason why I think they’re idiots.

For me, there are two really horrible things that come up in the podcast, that just want to make me puke:

  1. Their attitude towards Unit Testing
  2. Their (mis)understanding of the Single Responsibility Principle

I was going to try and tackle these separately, but they’re fairly intertwined. Or, at the very least, you’ve got to tackle them in reverse.

The Single Responsibility Principle

The Single Responsibility Principle (SRP) basically says that each class in your code has only one responsibility. You probably could’ve worked that out from its name. Here’s what Joel and CHG have to say:

One of the SOLID principles, and I’m totally butchering this, but, one of the principles was that you shouldn’t have two things in the same class that would be changed for a different reason. Like, you don’t want to have an Employee class, because it’s got his name which might get changed if he gets married, and it has his salary, which might get changed if he gets a raise. Those have to be two separate classes, because they get changed under different circumstances. And you wind up with millions of tiny little classes, like the EmployeeSalary class, and it’s just… (laughs) idiotic! You can’t build software that way!

What? “You don’t want to have an Employee class”?? Have you got any idea how idiotic that sounds???

The SRP doesn’t say anything like what they claim it does. Of course you want an Employee class, and you want it to be responsible for the things an employee has and does. So an employee HAS-A name, and HAS-A salary, and the Employee class should have those things too. But the point is that the Employee class is only responsible for having the things an employee has, and should not and will not and does not take on any of the responsibilities of those objects contained within. So the Employee class isn’t responsible for verifying that the name is valid; that’s the job of the Name class. Nor is it responsible for verifying that the salary correct; that’s the job of the Salary class. But you still need an Employee class to bring them all together and be representative of the employee. And the only reason for changing that Employee class is because either what an employee has or what an employee does changes.

That’s the Single Responsibility Principle. That’s why when a class has a single responsibility it only has a single reason to change; when its responsibility changes.

And, yes, I do agree that, when you’re writing your Employee class you may just have a String for name and an Integer for salary. That’s fine; knowing when to do things right and when to do things now is one of the hardest parts of the art of software engineering. But when you make those trade-offs, do so knowingly. Understand what you’re gaining, and what you’re throwing away.

Oh, and make sure that as soon as something goes wrong with the salary or the name, or anything else that you’ve embedded in the Employee class, or if you just get the faintest whiff of code-reuse, (say, when you’re adding the Director class), you make sure that you Do The Right ThingTM and make sure that you turn that wannabe class into a Proper Boy (© Mr Rob Allen).

Unit Testing

So, what about Unit Testing? First off, what have Joel and CHG got to say about it?

…the real problem with unit tests as I’ve discovered is that the type of changes that you tend to make as code evolves tend to break a constant percentage of your unit tests. Sometimes you will make a change to your code that, somehow, breaks 10% of your unit tests. Intentionally. Because you’ve changed the design of something… you’ve moved a menu, and now everything that relied on that menu being there… the menu is now elsewhere. And so all those tests now break. And you have to be able to go in and recreate those tests to reflect the new reality of the code.

I’m sorry, what? You wrote a unit test that broke when a menu moved? Let me rephrase that – you wrote a test that broke when a menu moved, and you called it a unit test?

Urgh.

As some of you know, I’ve some fairly strong (some might say militant) views on Unit Testing; and I’ll be the first to admit that those views are being constantly refined with every line of code I write, and every article on Unit Testing and TDD I read, and every Agile lecture I go to, and every argument I have with Matt in the kitchen, and every time Rob points out the gaping hole in a wild generalisation that I make.

But the fact remains that the biggest problem I have with Unit Testing is that people don’t understand what a unit test is.

This really is a whole post (book?) of its own, and I’ve spent long enough on this one as it is. So I’ll try to keep this short. What they describe isn’t a unit test. If 10% of your unit tests break because you’ve changed the position of an item in a menu, then either you’ve only got 10 tests and one of them is the unit test for the menu layout, or you’re not writing unit tests. And you shouldn’t be writing unit tests for menu layouts anyway.

Everything that’s unit-testable should be unit-tested. That’s the bottom line. If it should be unit-tested and isn’t, then fix it. Whether it’s because it’s designed badly, or you just didn’t bother to write the tests in the first place, just Do The Right ThingTM and write the tests.

So, next time on SSEQotW; Everything that’s unit-testable should be unit-tested – but how do I know what’s unit-testable?

Oh, and finally, one last thing from the Joel-and-CHG-show; excesive use of ISP is probably Code Smell. And if you’ve no idea what I mean, and you’ve read the podcast transcript,  then you’ll have to wait till next time, or accost me in the kitchen and I’ll let you know.

Advertisements
2 Comments leave one →
  1. October 8, 2009 8:54 pm

    Here’s some proof Spolsky has views that differ from yours and mine (and is therefore an idiot).
     

Trackbacks

  1. How to Interview a Programmer « dantwining.com

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: