Before I start, let me do a volume check.  If I continue to talk this loud, can everyone understand me? The title of this talk used to be “Software Craftsmanship,” but I found out that was already a book title, so now it’s called “The Software Craftsman.”

By way of introduction, let me say that...

this topic based on an article that recently appeared in Software Development magazine.  There is a column in it called “The Craftsman” and this article appears to be the first installation. It happens to be an excerpt from a book that is soon to be published on agile software development.  Perhaps the Developers SIG could get a copy for review.

Did anyone look at the Smalltalk version of the code?  I’m going to hand out a couple of copies of that for later. I also have most of these documents on this computer, so we can look at them later if anyone has questions.

The article has a cast of three characters: Alphonse the apprentice, Jerry the journeyman, and Mr. C. the master craftsman. This is set up parallel to the old guild system.  At some point the author will probably contrast this system to a group of computer scientists or a department of software engineers and conclude that craftsmanship rather than science or engineering is lacking in software development. 

Anyway, episode one is about Alphonse’s first day at work in which he gets oriented.

He is instructed to program the Sieve of Eratosthenes which is an algorithm used to find prime numbers and while he’s at it write some unit tests. He does receive some additional requirements.  He is to program in Java and have it ready right after lunch.  The program will be checked for adherence to Mr. C’s standards, which have not yet been defined, unfortunately. So, Alphonse codes away and is finished long before lunch, doing his best to meet some unspecified standards.

After lunch he turns in his work to Jerry, the journeyman, who looks at it for a couple of minutes, then says that it will get them both fired. The first problem that he finds is that one method is trying to do too many things.  This might be called the long method problem, or the schizophrenic method or the jack-of-all-trades method.

Tonight we will try our own hand at a code review and see what other problems lurk in Alphonse’s program. We will then look for some simple ways to improve the code short of throwing it out and rewriting. As any changes are made, we will run some tests that are designed to ensure that the code still works despite our changes.

and when possible we’ll try out some tools that are designed to automate some of these tasks.

Onto the code review...

The information I have on code reviews is mainly from this first book “Handbook of Walkthroughs, Inspections, and Technical Reviews” which was originally published in 1977.  It is the third edition that is from 1990. Some more up-to-date ideas on reviewing code is contained in this book on extreme programming by Kent Beck.  The code reviews there are a side effect of pair programming.  This is the “review early and often” approach.

At one meeting someone said that they had done reviews in real life. Is that person here?  Can you tell us about it? Is anyone really doing pair programming that is part of XP or agile software development? I remember doing it for a programming course in Germany, but that was probably because there was a lack of computers and because they didn’t think that we could solve the problems on our own.  In retrospect, it was probably good practice.

The authors of this red book, Freedman and Weinberg, list some reasons that code should be reviewed. On the one hand, the review can point out needed improvements in a product.  This probably happens a lot. On the other hand, it can identify places where improvement is not needed.  I can say that I not too infrequently spent too much time perfecting something, often a user interface, when it is not really necessary. The last reason here is related to process improvements.  The work comes out more predictable as more people have contributed.

Having multiple people see and understand each part of the code gives you an insurance policy in case someone jumps ship or goes on vacation, etc. Tests for any bugs that are found can be added to unit tests so that the bugs do not resurface. Sometimes programmers working in isolation come up with parallel solutions where a single implementation would be advantageous.

If you are reading someone else’s code then it should not be difficult to understand because of formatting differences.  Get everyone to write in a reasonably similar style or get programs that will convert between styles. The code that I put on the web does not satisfy this guideline.  That code was exported as RTF text with syntax highlighting.  I added line numbers with Word, but when Word converted to HTML, the line numbers were lost.  Furthermore, the paper version that I’m handing out is only in black and white.  Sorry. In this red book they explain how reviews helped achieve clean compiles.  This was very important in the days when punch cards were used, but today the cost of having humans check for syntax errors is much higher than having the compiler do it. In general, the code should be understood by everyone before the review begins.  This is not very important in the case of this toy example code. The book recommends against correcting problems during the review.  It probably isn’t a good idea to have someone making code changes while lots of other people are waiting. Lastly, don’t tread on people’s feelings.  One way to do this is to identify good things in the code.  This should also encourage others to adopt good habits.

Some organizations have checklists of items that should be covered.  There are many style manuals that contain lists and pattern guides that demonstrate recommended ways of doing things.  There is, for example, a book called “Smalltalk Best Practice Patterns.” Sometimes it might not be clear that the code does what it is supposed to do, so design documents might come in handy. Often a report must be filled out that indicates the outcome of the review.  The outcome might be

There are several tasks to perform at the review depending on how many people are present, how formal the meeting is, etc. Make table on board with columns for problems and room for solution to be added later.

The next topic is refactoring.

The information that I have on refactoring comes from the book by the same title.  It is an excellent book and we could take an entire meeting to discuss it.

Basically you are satisfied with what the program does, but not how it goes about doing it.  You set about to change it internally, but not so that any external behavior is modified.

In this case the design that is being improved is often very detailed design that is not necessarily part of any design document. Simplifications are often made to things like spaghetti code.  Sometimes they have to do with realizations made after the first coding pass.

When you are changing the code anyway, you may as well improve it a little, too. If others are goings to look over your code, then it should be as understandable as possible. Fowler says that someone told him that the first time you do something, you just do it.  If you have to do it a second time, you don’t like it, but do it anyway.  The third time you do something similar, you refactor, probably with an emphasis on reuse.  This he calls the third strike.

Fowler has a long list of problems with code.  He calls them bad smells, because something is rotten and stinks.  Here are just a couple of examples. A long method may be trying to do too many things at once and should instead be coordinating several other methods that whittle away at the task.  Does anyone have a recommendation on how long a method should be? The author compares comments to deodorant.  He says that they cover up the smell, but don’t really remove it. A lazy class is one that doesn’t do enough work.  Our example might be like this.  It has only one method and that is a class method.  There are no instance or even class variables.

A potential solution to the long method problem is to identify parts of its code that can stand on their own as methods.  Replace the original code with a call to the method.  This is rather difficult to automate, of course. The second example is pretty much common sense.  Despite this, there might be an example in our Sieve code.  If the magic number appears several times, it may be difficult to tell whether there is only one number or several. Renaming some variables, especially instance variables, may be quite complex.  You don’t want the renamed variable to mask variables in super classes or possibly get in the way of method variables.  It may have to be changed in header files and implementation files.

Refactoring can be done by hand, of course, but it is an even more powerful technique with the help of tools. There are refactoring tools for at least Smalltalk and Java.  I don’t have any experience with those for Java, so we’re going to use Smalltalk tonight.  Has anyone else used tools to refactor Java? A related tool involves testing.  As soon as we change anything we’re going to want to test the changes.  The tests are really a prerequisite for refactoring.  For now I’m going to gloss over how this works because it is the next topic.

Lets look a little closer at how this testing works.

Once upon a time Kent Beck wrote a little article on testing.  What he did was pretty much state the obvious, but he was influential enough that his idea became a standard and has been adopted by many people. There is a web site where you can download the unit testing software for lots of different language.

Has anyone already used this software?

The original idea is that classes should contain their own tests.  It has turned out that the testing framework in Smalltalk at least works a little better when this in not actually the case.  Test classes are kept separate from the classes being tested.

In the example there really isn’t a fixture, since there are only class methods and no variables.  If you were testing sets, an example would be the empty set or the set of even numbers less than 100. The test case in the example is really the collection of maximum values 0, 2, 3, and 100. Results are often checked with assertion statements.  In Java there is an assertEquals method, but Smalltalk often uses should: and shouldnt: Lastly, tests can be grouped together into test suites.  In this example, the test suite is generated automatically using language reflection capabilities, but it is also possible to make suites in code.

You’ll see from the user interface that there are three possible outcomes to a test case: the test can pass, fail, or result in an error. If an error happens on the customer’s computer, it fill be a fault and bad things will happen.

You will run the tests often, so they should work automatically.  No user input is allowed. Tests should be sophisticated enough to detect errors themselves.  The unit tester should not have to look over the output. You can’t test everything.  Don’t forgo testing for lack of completeness.  Fowler’s advice is that it is better to write and run incomplete tests than not to run complete tests which are never finished.

What can I say to summarize?

According to the dictionary, a craftsman is a person who makes or manufactures with skill and attention to detail. What we have been talking about tonight isn’t so much science or engineering, but the skills of reviewing, refactoring, and testing. To this extent I think that the guild analogy that Martin uses in his article is a good one.