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
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:
var autoCreate = true;
var useCache = false;
GetRecord(autoCreate, useCache);
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:
GetRecord(autoCreate: true, useCache: false);
Or if this syntax is not available, like this:
GetRecord(true , false );
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:
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.
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:
public Record GetRecordNoCache()
{
return database.GetRecord();
}
Our other methods then become:
public Record GetRecord()
{
return cache.GetRecord() ?? GetRecordNoCache();
}
public GetOrCreateRecord()
{
return GetRecord() ?? new Record();
}
Now instead of:
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:
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.