Click here to Skip to main content
65,938 articles
CodeProject is changing. Read more.
Articles / Languages / C#6.0

Boolean parameters and code readability

4.82/5 (25 votes)
25 Apr 2017CPOL1 min read 26.7K  
Annotating parameters vs Rewriting methods for code readability

In response to Annotating constant parameters (Mar 31, 2017).

The example given in the article above tries to address the ambiguity of parameters in methods calls. If a call such as

C#
GetRecord(true, false);

Is used, the reader has no idea what true or false mean in this case. Usually they may have some indicator if these values are variables or parameters such as:

C#
// Example 1: Using variables
var autoCreate = true;
var useCache = false;
GetRecord(autoCreate, useCache);

// Example 2: Using parameters
public void ProcessRecord(bool autoCreate, bool useCache){
    GetRecord(autoCreate, useCache);
    ...
}

However the author suggests that if this is not the case then you should annotate your parameters like this:

C#
GetRecord(autoCreate: true, useCache: false);

Or if this syntax is not available, like this:

C#
GetRecord(true /* autoCreate */, false /* useCache */);

On the face of it, I don't disgree that this makes the code a little more readable, however if you can, you should go further. Imagine the GetRecord function itself:

C#
public Record GetRecord(bool autoCreate, bool useCache)
{
    Record record = null;
    if(useCache){
        record = cache.GetRecord();
    }else{
        record = database.GetRecord();
    }
    if(autoCreate && record == null){
        record = new Record;
    }
    return record;
}

Instead of using arbitary parameters, I would instead break this down into multiple methods to make it more readable and useable.

C#
public Record GetRecord(bool useCache)
{ 
    if(useCache){
        return cache.GetRecord();
    }else{
        return database.GetRecord();
    }
}
    
public GetOrCreateRecord(bool useCache)
{
    var record = GetRecord(useCache);
    if(record == null){
        record = new Record;
    }
    return record;
}

We now have two simpler methods that do what their name implies. However the parameter for useCache is an arbitary boolean that could be eliminated. As a consumer of the GetRecord function I shouldn't have to specify where it's getting it's record from.
There are a few solutions to this depending on what is required. The simplest is to create another method. I would usually think that using a cache would be implied unless otherwise specified, so I would create another method as follows:

C#
public Record GetRecordNoCache()
{       
    return database.GetRecord();
}

Our other methods then become:

C#
public Record GetRecord()
{       
    return cache.GetRecord() ?? GetRecordNoCache();
}

public GetOrCreateRecord()
{
    return GetRecord() ?? new Record();
}

Now instead of:

C#
public Record GetRecord(bool autoCreate, bool useCache)
{
    Record record;
    if(useCache){
        record = cache.GetRecord();
    }else{
        record = database.GetRecord();
    }
    if(autoCreate && record == null){
        record = new Record;
    }
    return record;
}

GetRecord(autoCreate: false, useCache: true);
GetRecord(autoCreate: false, useCache: false);
GetRecord(autoCreate: true, useCache: true);

We have:

C#
public Record GetRecordNoCache()
{       
    return database.GetRecord();
}   
public Record GetRecord()
{       
    return cache.GetRecord() ?? GetRecordNoCache();
}   
public GetOrCreateRecord()
{
    return GetRecord() ?? new Record();
}

GetRecord();
GetRecordNoCache();
GetOrCreateRecord();

Which is another step forward in making our code more readable.

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)