Monday, 11 May 2009

Readability in Method Calls

I generally prefer to call methods using variables rather than literals or expressions as arguments . I don't follow this religiously but it is especially helpful when calling API methods that have boolean or object parameters that can be true/false or null. For example,

AuthorizationRuleCollection rules = 
fileSecurity.GetAccessRules(true, true, typeof(NTAccount));

This is a fairly mild case but what does true mean? I've no idea. It's worse when we see this kind of thing,

DoSomething(width, height, null, null, name, false);

Here, I have no clue what null or false represents.


What I do in such cases is replace the null or boolean with a local variable. So, in the first case I write,

bool includeExplicit = true;
bool includeInherited = true;
Type type = typeof(NTAccount);
AuthorizationRuleCollection rules =
fileSecurity.GetAccessRules(includeExplicit, includeInherited, type);

Suppose includeExplicit or includeInherited is false? Then I write,

bool includeExplicit = true;
bool includeInherited = true;
Type type = typeof(NTAccount);
AuthorizationRuleCollection rulesCol =
fileSecurity.GetAccessRules(!includeExplicit, includeInherited, type);

(The only problem with the last case is that it easy to miss the ! operator. This is one of the shortcomings of the C-family languages. A not keyword would have been preferable.)


Even outside the cases discussed it is generally more readable to use variables instead of literals or expressions as arguments.

2 comments:

  1. Here's a thought - in C# 4.0 you could achieve this with named parameters, eg fileSecurity.GetAccessRules(includeExplicit: true, ...

    Best of both worlds - clearly readable, and without cluttering with single use variables. Agree that prior to C# 4.0 your method is best practise.

    ReplyDelete
  2. Hi Chris, yes I'm aware of the new C# 4.0 feature and will deinitely use it in such cases if and when I get to use C# 4.0 for real.

    ReplyDelete