Using parameter names to write self-documenting and self-diagnosing code, part 2

Identify the ultimate pre-condition.

Part one in this mini-series is found here.

Here is a simplified extract of a program I am working on right now.

A principal method:
public void process(Dto dto, PresumptiveOrder presumptiveOrderOrNull) {
    // some code
    commonExecute(dto, appointmentOrderOrNull);
    // some code
}

commonExecute:

protected void commonExecute(Dto dto, PresumptiveOrder presumptiveOrderOrNull) {

    dto.to(appointmentOrderOrNull);

}

to:

public void to(PresumptiveOrder presumptiveOrder) {

    presumptiveOrder.setProperty(getProperty());

    // set more properties

}

Now, this becomes interesting. The method process states that it is ok for it to get null as an order parameter. Then, it forwards this to  commonExecute, which also find that to be fine, and forwards it on to the to method in the dto, which is neutral as to what kind of parameter it will welcome. And that's where things start to get exciting.

For what happens if the order parameter is actually null, as allowed by the caller chain? Well, the statement presumptiveOrder.setProperty(getProperty()); will of course crash with a NullPointerException. So we have a problem to tacle. But how?

I will not discuss here the option of checking the parameter for null and then attempt some hopefully correcting or compensating actions. I frequently meet that attitude, sometimes under the name of Defensive Programming. I don't believe in that approach, but that's the subject for a different post. Instead, I promote an offensive style, based on the assumption that should this situation ever occur. it must be the result of a programming error (or a design error). Then, it becomes important to identify and correct that error in the code as soon as possible.

Two pieces are needed for this kind of errors to be avoided. Firstly, the caller of the method must know that a null parameter is illegal. Secondly, the method itself must detect and signal when it nevertheless does occur.

The first piece is solved by including this restriction in the name of the formal argument. So, I start by renaming presumptiveOrder to presumptiveOrderNotNull. Now, this important, semantic information is signalled to the caller from whereever he is calling this method, eg. through a tooltip like this:

The second piece is solved by including a precondition verification, as discussed in part one of this mini-series. The implementation of the method now becomes

public void to(PresumptiveOrder presumptiveOrderNotNull) {

    DbC.precondition(presumptiveOrderNotNull != null, "The order should not be null");

    presumptiveOrderNotNull.setProperty(getProperty());

    // set more properties

}

So far, so good.

Now, what about commonExecute calling to? With input argument presumptiveOrderOrNull, allowing an input value of null, there is no guarantee that the precondition for to is satisfied. So what is gained? Of course, what is gained is that it now becomes obvious that the input condition ...OrNull will not do, so it must be corrected. This will then push the stronger condition ...NotNull backwards through the caller chain, from the leave method, setting the ultimate pre-condition, to the principal method. The resulting calling chain then becomes

A principal method:
public void process(Dto dto, PresumptiveOrder presumptiveOrderNotNull) {
    DbC.precondition(presumptiveOrderNotNull != null, "The order should not be null");
    // some code
    commonExecute(dto, appointmentOrderNotNull);
    // some code
}

commonExecute:

protected void commonExecute(Dto dto, PresumptiveOrder presumptiveOrderNotNull) {

    DbC.precondition(presumptiveOrderNotNull != null, "The order should not be null");
    dto.to(appointmentOrderNotNull);

}

to:

public void to(PresumptiveOrder presumptiveOrderNotNull) {

    DbC.precondition(presumptiveOrderNotNull != null, "The order should not be null");

    presumptiveOrderNotNull.setProperty(getProperty());

    // set more properties

}

Now, the principal method process clearly signals that somewhere along the chain, there is a requirement for the order parameter not to be null. This allows this condition to be verified already at the outset, eliminating the risk for potentially tricky problems later on. And by requiring and verifying this input condition, each caller along the chain can confidently call the next method, knowing that ist pre-condition is satisfied.

In the next, and last, post in this mini-series, I will discuss the case of a polymorphic situation where some of the implementations have a stronger pre-condition than others.

Using parameter names to write self-documenting and self-diagnosing code, part 1

Since some time, I have taken on the habit of letting variable names be their own comments. That turns out to be an outstanding quality and documentation aid, that I would like to share with you. By far the most common is naming method parameters following the patterns parameterOrNull and parameterNotNull.

Assume you have a method for validating order details before processing:
public Validation validationResult(PresumptiveOrder presumptiveOrder)

Of course, you expect presumptiveOrder to be non-null. To document that, I then rename it to presumptiveOrderNotNull.

As soon as I see a name like that in the formal argument list, I know that there is a precondition to verify. Now, Design by Contract is not implemented in the common languages, with a reservation for C#'s Code Contract, that I have merely looked at superficially. For me, it seems too heavy-weight. but I may give it another try if prompted to. So instead, I am using my own, light-weight, 80/20 solution, which is as follows.

• Make a class DbC (short for Design by Contract). I am normally against abbreviating class names, but this on is an exception. This class is not part of the functionality of the system, it is a quality assurance measure. I make an effort for the precondition verification to be as discrete as possible, and the abbreviation DbC is part of that.
• Make a static method

public static void precondition(boolean condition, String message){
    if (!condition){
        throw new PreconditionViolationException(message);
    }
}

• Make an exception (this is for Java. For C#, just inherit from Exception)

public class PreconditionViolationException extends RuntimeException {

    public PreconditionViolationException(String message){

        super(message);

    }

}

Voilà. You have made yourself a low-profile design by contract framework that will do in the vast majority of cases. This is what I use myself, and it serves me well. So how is it used?

Back to the example, I want to verify that the parameter presumptive order is not null:

DbC.precondition(presumptiveOrderNotNull != null, "The presumptive order should not be null");

Do I hear you screeming at me? presumptiveOrderNotNull != null looks very much redundant. The variable states that it is not null, and then I verify that it is not null. That should be quite evident, shouldn't it?

Well, not quite. You see, the point is this. The variable name states a requirement on the parameter, not a fact. This requirement is called a pre-condition. What this call to DbC.precondition does is to verify that the pre-condition holds, so that the code in the rest of the method can safely rely on it.

Oh, by the way. It is of utter importance that the calculation of the actual condition parameter, in this case presumptiveOrderNotNull != null, does not have any side effects (or at least not any side effects observable from the outside, but that is a risky statement to make). That is obviously the case here.

The next post in this mini-series discusses how to identify and implement the ultimate pre-condition.

Anemia or not anemia, that's the question

Bottom line:
A refactoring transition from an anemic to a rich object model is possible, painful, and—at least for me—necessary.

I'm currently working on an anemic, Struts based J2EE POJO solution that is a few years old. As I understand it, it goes by the book, with class name patterns like OrderLogic, OrderDTO (Data Transfer Object), OrderDAO (Data Access Object) and Hibernate annotated OrderVO (Value Object). They implement an anemic model with logic classes and DAOs containing purely code, and DTOs and VOs containing purely data, close to a Table Module architecture, with VOs acting as data records, which is basically a well structured Transaction Script model: Plain old data based, structured programming.


Oo freak as I am, this frustrates me, especially as there is leakage of responsibility between the classes. On the positive side, there are some good architectural efforts, with OrderLogic managing the high level request for orders, delegating the pure db-access to the DAO. Operations on the resulting OrderVO is mostly also found in OrderLogic, but also elsewhere, for instance in Struts Action classes, as well as in other XxxLogic classes. And there is duplication. And, surprise, surprise, all this is also to be expected, "by the book".


For my brain to function properly, I need to apply the SRP and the IE principles, so as to find my operations close to my data. I also need to support the changes I make with TDD. All this requires a rich, non-anemic domain model. Successively, instead of one big folder (and namespace) common for everything relating to say customer and order management, I create folders for three architectural layer, like Presentation Layer (common.PL), Domain Logic (common.DL) and Data Access (common.DA). To start with, PL contains the DTOs and related stuff, DL contains the new domain objects and other domain layer logic, like class repository objects, mostly refactored out of the Logic classes. Finally, DA contains DAOs and DVs. 


At least, that's what I thought.


Actually, after some refactorings, I am heading in a slightly different direction. In my search for how to design my domain objects, I have investigated three different models:

1. inherit from the VOs
2. delegate to the VOs
3. promote the VOs to rich domain objects

I chose the third one, So I am successively moving the VOs to the DL (Domain Logic) package, removing the VO postfix in the process, and successively moving logic from Logic classes and elsewhere into them. That raised the dilemma of the DAOs, managing the low level Hibernate access and actually creating and saving the former VOs. They cannot be located in an architectural layer below the objects they are to manage. So now, I have promoted the DA package to the domain logic layer, and I am moving the Repository classes into DA, the difference between them and DAOs being that DAO contains the Hibernate mapping, whereas the repositories handle requests for domain objects from the rest of the system.


When time allows, I will add a figure here to explain this more clairly. For the time being, I am curious to see where this strategy takes me.

Comments considered harmful 2

I am getting "Too many open files" errors. So I check every usage of files and streams, and make sure I always close them. Here is a snippet I found that was not closing the out stream after use (empty lines and failure detection suppressed):

JspWriter out = pageContext.getOut();
if (errorMessageToUser == null)
{
    drawCalendar(out);
} else {
    out.println(errorMessageToUser);
}

So I add a line closing the writer, like this:

JspWriter out = pageContext.getOut();
try {
    if (errorMessageToUser == null)
    {
        drawCalendar(out);
    } else {
        out.println(errorMessageToUser);
    }
} finally {
    out.close();
}

Of course, this broke one of the golden rules, saying that you only should close files that you have opened yourself. This writer was opened by the jsp framework, so I got a "Stream already closed" error somewhere else when jsp wanted to close it.

I need some way to avoid redoing the same mistake. My first attempt was to add a comment:

JspWriter out = pageContext.getOut(); // Don't close
if (errorMessageToUser == null)
{
    drawCalendar(out);
} else {
    out.println(errorMessageToUser);
}

Now, I have a problem with this kind of comments. It is localized with the declaration, so to see it, you have to go back there. My recommended practice today is to try to incorporate the comment into the variable name. So this is my present version of this code:

JspWriter outDontClose = pageContext.getOut();
if (errorMessageToUser == null)
{
    drawCalendar(outDontClose);
} else {
      outDontClose.println(errorMessageToUser);
}

I find this pattern outstanding. By being its own comment, the code tells me about its semantics, its rules of usage. And, best of all, it is not localized to the point of declaration. I am reminded of the rules every time I see the variable name.

Comments considered harmful

At school, I was taught to comment my programs well. Today, I wonder why I should.

Every comment relating to the code itself is redundant. Redundancy creates inconsistencies, and inconsistencies are a serious threat to code quality. So I prefer not to comment.

"Hey, come on! How do you expect anybody to understand your program if you don't comment it?" I hear being shouted at me. I will ask the opposite question: Why should I write such a lousy program that it needs comments to be understood?

I worked in a group where the coding guidelines said: Comment every class, comment every method. I refused. I mean, as an example, look at this getter, generated by the Refactor -> Encapsulate Fields... operation of NetBeans:

    private String name;
    /**
     * @return the name
     */
    public String getName() {
        return name;
    }

What does that comment add that I did not already learn from the method name? So rather than commenting a method, I prefer to use the comment,  kind of, as the method name. That guarantees consistency and also propagates the "commented" intent to every place the method name is referred to. Far superior to a localized comment!

Martin Fowler once said: "Any fool can write code that a computer can understand. Good programmers write code that humans can understand." I strive to be one of them.

Iterations and increments iterated

It seems hard to get this right. Jeff Patton uses Mona Lisa to explain it. Alistair Cockburn uses Michelangelo and van Bruegel the elder, stating that "Incremental means adding, iterative means reworking" (my emphasis). That would be in the product dimension. Thomas Nilsson, in a Swedish agile forum, writes "In my world, incremental relates to the product dimension, whereas iterative relates to the process dimension" (my emphasis).

To summarize, I find it helpful to consider iterations and increments in both of these dimensions.

• An iterative process would be like any agile process. You repeat the same development activities over and over. That's what we like to do.
• An iteration in the product dimension would be, as Cockburn points out, improving quality or functionality of the product. That is certainly legitimate in a lot of situations. You make a tentative solution — not necessarily a prototype — and you present it. It may be accepted or rejected. But even if accepted, the customer may want to iterate on it to improve it. Tom Gilb calls it evolutionary development. And we may want to iterate on it, refactor it, to improve its internal quality.
• An increment in the process would imply doing more and more things in each iteration, like documenting more, testing more, designing more, or even adding new steps. This happens in many situations where thing go wrong. Then you add rules or steps to go through to prevent that from happening again. Isn't that how heavy processes emerge?
• Incrementing the product would be adding new features to it. That's what we normally do.

All of this may be summarized in a small table.

Is this distinction helpful? Well, I think it is always helpful to understand the words you are using.

So, within the framework of an iterative process, we both iterate on the product and we increment the product.

Does all of this assume an iterative process in the first place? I don't think so. Look at a typical waterfall development process. While deployment may be neither iterative nor incremental, I am sure that in most cases the development is de facto incremental, despite the linear process. The developers are even likely to iterate over the (still undeployed) product. So the product is developed both iteratively and incrementally. But the process is not iterative.

And I find that in RUP, for instance, the process is iterative, and you are supposed to build the product incrementally. But RUP does not encourage iterating on the product itself. To me, that's one important factor that makes it very waterfallish and inapt to adaptive development.

To implement the agile values, the process needs to be iterative, and you need to both iterate on the product and increment the product during your process iterations. Waterfall fails in not being an iterative process. RUP fails in not iterating on the product.

The Agile Team Room

During a recent training course, we discussed the agile team room, especially the whiteboard space for modelling. Fortunately, Mike Cohn just published a blog, listing the essential features for The Ideal Agile Workspace:

• Big Visible Charts
• Additional feedback devices
• Everyone on your team
• The sprint backlog
• The product backlog
• At least one big white board
• Someplace quiet and private
• Food and drink, and finally
• A window
  

The comments link to several implementations of this list. For instance, one of the comments link to a tour through the msdn Patterns and Practices Lab and a peek inside the p&p Team Room. Of course, these are quite advanced implementations of Mike's list, but show how important these companies find the physical environment.