How to Reduce Cyclomatic Complexity Part 3: Avoiding Optional Parameters

by Zoran Horvat
April 21, 2015

In the previous two articles we have seen how we can reduce cyclomatic complexity of code by never returning null references back from methods. That measure helps the caller operate on resulting objects without testing them against null.

All objects are treated in the same way and there is no branching based on null/non-null division between references. For more details, refer back to previous articles: Null Object Pattern and Special Case Pattern .

In this article, we will turn our attention to branching which is caused by special parameters passed to the method. Most notorious case are parameters which may be null to indicate special conditions. Implementation then typically boils down to testing whether parameter is null or not and then taking one course of action or the other.

Optional Parameter Example

We will continue fixing the same e-commerce application which was used to demonstrate refactorings in previous articles from this series.

Suppose that our website supports referrals: Registered users are eligible for a discount if they bring in another user.

This requirement can be implemented in application services by providing an optional referrer name as part of the user registration method:

public interface IApplicationServices
{
    void RegisterUser(string userName, string referrerName = null);
    ...
}

In this way, we are supporting both kinds of registrants: Those that come with the referrer and those that don’t.

This feature can easily be implemented in the domain services:

public class DomainServices: IDomainServices
{

    private readonly IUserRepository userRepository;
    private readonly IProductRepository productRepository;
    private readonly IAccountRepository accountRepository;

    public void RegisterUser(string userName, string referrerName)
    {

        User user = new User(userName);

        this.userRepository.Add(user);

        Account account = new Account(user);
        this.accountRepository.Add(account);

        if (referrerName != null)
        {
            User referrer = this.userRepository.Find(referrerName);
            if (referrer != null)
                user.SetReferrer(referrer);
        }

    }
    ...
}

This implementation is relatively straight-forward. But it exhibits several problems.

Identifying Problems with Optional Parameters

One obvious problem is that it is complicated, or, more formally speaking, it has high cyclomatic complexity. Setting the referrer boils down to nested if statements.

Imagine what it would mean to test this method. We would have to cover one case with referrerName parameter set to null. Then, another case with the same parameter set to non-null, but such that user repository returns null because such user does not exist. Finally, we would have to test the most elaborate case in which referrerName is non-null, userRepository finds the corresponding record and produces the non-null referrer object and finally the SetReferrer method on the newly registered user can be invoked.

If-then-else logic is the symptom we almost always find together with optional method parameters. But as I said, that is not the only problem here. Much more profound problem is that this method implementation is branching based on null test performed on the method parameter.

Put the other way, method performs one operation or the other based on whether its concrete parameter is null or not. You may ask why this is the problem. The problem is that the caller might make a different decision based on the same parameters.

Imagine this situation. Caller has passed in a null reference instead of the referrer name. This immediately indicates to the domain service that there is no referrer, so the domain service takes the alternative course of action. However, caller may think that the parameter was non-null. It is easy to make a mistake like this – suppose that the caller has obtained the referrer name from its own caller and didn’t check whether it’s null or not.

Net result of such misunderstanding is that the caller has performed operation as if the referrer is there. Domain service has performed operation as if the referrer is not there. They both commit their changes, report back success and life goes on. Just to find a few weeks later that all the receipts issued in the meanwhile were incorrect because the referral discounts were not applied along the way.

Solving the Optional Parameters Problem

One way to deal with optional parameters is to completely avoid them.

Make sure that every method clearly communicates its intention. This is done by properly naming it, and by properly selecting its parameters. All parameters are required for the operation. There is no room left for optional parameters.

If there is another similar scenario, define another method which only covers that scenario. Parameter list for that method can be shorter than the first one.

Sometimes, these two methods will share the same name. This is the case where parameter list clearly enough communicates the intention of the method. Sometimes, however, method names will differ to indicate subtle differences in scenarios they cover.

Applied to e-commerce application, we would see two RegisterUser methods in the application services:

public interface IApplicationServices
{
    void RegisterUser(string userName);
    void RegisterUser(string userName, string referrerName);
    ...
}

The first one just registers the user. The second one registers the user and also points to the user who referred the website. From these two method signatures it should be obvious that executing the second causes more changes to be made to the system compared to executing the first one. And it is so – the second method will search for the referrer and make it eligible for the referral discount.

Template Method Pattern for Slightly Different Scenarios

Programmers often reach out for optional method parameters just because implementation is almost the same in both scenarios. One if statement at the end of the method is typically what it takes.

But as we pointed out, the problem is that alternate scenario might not be perceived by the caller and then spooky defect would appear.

Let’s see previous implementation of the user registration in application services and it will become obvious where the problem lies:

public class ApplicationServices: IApplicationServices
{

    private readonly IDomainServices domain;

    public void RegisterUser(string userName, string referrerName)
    {

        if (IsDownForMaintenance())
            return;

        this.domain.RegisterUser(userName, referrerName);
        Session.LoggedInUserName = userName;

    }
    ...
}

RegisterUser method of the ApplicationServices class is doing several things. First, it is testing whether the application is down for maintenance. Then, it is contacting domain services to perform the registration. Finally, it is setting the session variable, indicating that newly registered user is logged in from now on.

Now that this RegisterMethod in the application services needs to be split into two, we have a slight problem. Only the middle part of the implementation varies, while all other operations remain the same regardless of the registration process.

We can solve the problem by enclosing the variation into an object and keeping everything else the same. Instead of invoking the domain services method, this RegisterUser method could invoke a lambda:

public class ApplicationServices: IApplicationServices
{

    private readonly IDomainServices domain;

    public void RegisterUser(string userName, string referrerName)
    {
        this.RegisterUser(userName, uname => this.domain.RegisterUser(uname, referrerName));
    }


    public void RegisterUser(string userName, Action< string > domainRegisterUser)
    {

        if (IsDownForMaintenance())
            return;

        domainRegisterUser(userName);
        Session.LoggedInUserName = userName;

    }
    ...
}

Now we have two RegisterUser methods. The public one looks the same as before. But the private one is a novelty.

Private RegisterUser method is the example of Template Method design pattern. It performs all the operations in the fixed order. But when it comes to passing on the call to the domain services, it just delegates to the lambda it has received as the argument.

In this way, we have closed the varying part of the user registration into an object - externally provided function. With this change, we are just one step away from the final solution in which registration process will be split into two independent scenarios:

public class ApplicationServices: IApplicationServices
{

    private readonly IDomainServices domain;

    public void RegisterUser(string userName)
    {
        this.RegisterUser(userName, uname => this.domain.RegisterUser(uname));
    }

    public void RegisterUser(string userName, string referrerName)
    {
        this.RegisterUser(userName, uname => this.domain.RegisterUser(uname, referrerName));
    }

    public void RegisterUser(string userName, Action< string > domainRegisterUser)
    {

        if (IsDownForMaintenance())
            return;

        domainRegisterUser(userName);
        Session.LoggedInUserName = userName;

    }
    ...
}

Now it’s all done. There are two public RegisterUser methods. One only receives the username and passes it to the single-parameter method in the domain services. The other receives both the new user and the referrer, and it passes the call to the corresponding two-parameter method in the domain services.

Separate Implementation for Significantly Different Scenarios

At this point, application services are calling one method or the other in the domain services. Two RegisterUser methods in the domain services are meant to cover two registration scenarios – with or without the referrer.

Before the change, domain services used to do two things in their RegisterUser implementation:

public class DomainServices: IDomainServices
{

    private readonly IUserRepository userRepository;
    private readonly IProductRepository productRepository;
    private readonly IAccountRepository accountRepository;

    public void RegisterUser(string userName, string referrerName)
    {

        User user = new User(userName);

        this.userRepository.Add(user);

        Account account = new Account(user);
        this.accountRepository.Add(account);

        if (referrerName != null)
        {
            User referrer = this.userRepository.Find(referrerName);
            if (referrer != null)
                user.SetReferrer(referrer);
        }

    }
    ...
}

Now we are fixing that and leaving each of the two methods with just one thing to focus on:

public class DomainServices: IDomainServices
{

    private readonly IUserRepository userRepository;
    private readonly IProductRepository productRepository;
    private readonly IAccountRepository accountRepository;

    public User RegisterUser(string userName)
    {

        User user = new User(userName);

        this.userRepository.Add(user);

        Account account = new Account(user);
        this.accountRepository.Add(account);

        return user;

    }

    public User RegisterUser(string userName, string referrerName)
    {

        User user = this.RegisterUser(userName);

        User referrer = this.userRepository.Find(referrerName);
        if (referrer != null)
            user.SetReferrer(referrer);

        return user;

    }
    ...
}

Both scenarios are now implemented in the straight-forward manner. There is no more reason for the implementation to branch based on parameter value.

Harvesting the Fruits of Mandatory Parameters

In order to demonstrate benefits we obtain from forcing mandatory parameters, let’s first look at the way in which presentation layer was registering users:

controller.RegisterUser("jack", null);
controller.Deposit(200);
controller.Purchase("book");

controller.RegisterUser("jill", "jack");

controller.Login("jack");
controller.Purchase("book");

This piece of code lets the user named Jack register, deposit some money and buy a book.

Later on, another user named Jill comes to register, but she refers back to Jack as the one who appointed her to the website. Internally, this operation will cause Jack to receive a referral discount.

Further down the stream, when Jack logs in again and buys another book, he should be able to see that the second book comes with the discount.

The problem with this piece of code is that null reference in the Jack’s registration call carries information that Jack has no referrer. From this line of code we cannot be absolutely certain about the consequences. Will the controller understand that null means no referrer? Or will it explode? Or will it do something completely different?

With changes made to the application and domain services, we can just omit that null:

controller.RegisterUser("jack");
controller.Deposit(200);
controller.Purchase("book");

controller.RegisterUser("jill", "jack");

controller.Login("jack");
controller.Purchase("book");

This time, there is no room for confusion. When Jack registers, he is the only one mentioned in the call. When Jill registers, both Jack and Jill are passed to the controller.

The primary benefit of having all parameters mandatory is clarity. There is no room for speculation what the method is doing.

Conclusion

In this article we have covered the topic of optional method parameters. Through a simple example, we have seen that optional parameter causes branching in implementation and, sometimes, causes confusion between the caller and the callee.

To resolve issues and make intentions more transparent, we have exposed separate methods for alternate scenarios. Each method is using all its parameters. There are no optional parameters and no branching based on whether the parameter is present in the call or not.

---