Don’t break the contract!

Defining a function is like defining a contract: you specify what the user of your function has to provide so that he gets the desired result. That is a basic principle in software engineering. In this post I will show what could happen if you break such a contract.

Let us have a look at the following function (you find the function in /cake/libs/model/datasources/dbo_source.php). It is a simple function that returns a string like “ORDER BY Post.title ASC”. This function works as expected.

/**
 * Returns an ORDER BY clause as a string.
 *
 * @param string $key Field reference, as a key (i.e. Post.title)
 * @param string $dir Direction (ASC or DESC)
 * @return string ORDER BY clause
 */
function order ($key, $dir = '')
{
    if (trim($key) == '')
    {
        return '';
    }
    return ' ORDER BY '.$key.' '.$dir;
}

But there is a little bug in it. If you provide “order” as $key parameter (because you have such a column in your database), your database will throw an error when it executes the generated sql. So we fix “our” function by quoting the content of the $key variable.

function order ($key, $dir = '')
{
    if (trim($key) == '')
    {
        return '';
    }
    return ' ORDER BY '.$this->name($key).' '.$dir;
}

That’s it, we have fixed the bug. Everything is fine, isn’t it? I guess you do not believe it as the topic of this post is not “bug fixing”. You are right. Theoretically, we have fixed the problem. But in reality, we have created a new problem, as you will see ;-)

Let us test “our” function. First, we omit the direction.

$this->Project->findAll(null, null, 'Project.name'));

That works, the debug output is:

... ORDER BY `Project`.`name`

And now let us add a direction to the order clause:

$this->Project->findAll(null, null, 'Project.name DESC'));

Oops, something has gone wrong:

... ORDER BY `Project`.`name DESC`

The reason for that problem is, that “our” function “order” is not used in the way it was designed for (probably due to practical reasons). It transpires that the $dir parameter is never used, instead the direction is included in the $key parameter, i.e. the $key parameter contains something like “Post.title ASC” (compare that with the function signature).

/**
 * Returns an ORDER BY clause as a string.
 *
 * @param string $key Field reference, as a key (i.e. Post.title)
 * @param string $dir Direction (ASC or DESC)
 * @return string ORDER BY clause
 */
function order ($key, $dir = '')

There are two possible solutions (probably more) to solve that problem. On the one hand all calls to the “order” function could be changed. And on the other hand the “order” function could be refactored so that it fits the needs of its users. I prefer the second option. But we will see how the CakePHP developers will solve that problem :)

2 Comments

  1. Nate
    Posted February 14, 2006 at 4:34 pm | Permalink

    So where do optional parameters fit into your contract theory? How about method overloading, vis-a-vis Java and C++?

  2. Posted February 15, 2006 at 9:50 am | Permalink

    Well, it is not _my_ theory ;-) What I have described is something we (as programmers) do every day: to define what parameters our functions/methods expect. That includes optional parameters, too.


Post a Comment

Required fields are marked *
*
*

%d bloggers like this: