Question

I'm looking for some advice about which line of code is more appropriate, readable, and maintainable.

1:

var result = mediator.Send(query).Result;

2:

var accountDetails = mediator.Send(accountFilter).Result;

The above code was taken from a Web API controller. The pros for each:

1:

  • If the query needs to change and return a different type, we don't have to remember to rename the result variable.
  • That also means code reviews won't be polluted with a bunch of files that have changed due to the renaming.
  • The job of a Web API controller isn't to know the details about what business logic is going on; it's just to invoke something and return it.
  • If you really want to know what the line of code is doing, you can look at the context surrounding it.

2:

  • It's easier to look at the line of code and understand what the line of code is doing.
Was it helpful?

Solution

Short answer: No

Long answer: No, vague names are far less maintainable than precise names. You won't rename things on a weekly basis (and if you do, you should ask yourself "What is the reason for this"). Changing the result type is a rather big change, so if I were to get a detailProxyBroker instead of the real accountDetails it would be kinda neat to know that at first glance...

Be as precise as you need be, so it you (or any random stranger who joins your team) can read & understand the code.

I would have a hard time to imagine what var result is I get frommediator.Send(query).Result;

var accountDetails I get from mediator.Send(accountFilter).Result; is much easier to understand. You do not need to include in the variable name that the accountDetails hold Name, Birthday and favorite colour - that is too much detail.

Heck, if not needing to change names is your goal just call it var a = b.Send(c).Result; you can stuff whatever you want in a, b and c... :/

So, to wrap it up, use variant (2) - your coworker (and you yourself) will be glad to have (nearly) self-documenting code, when you need to work with that piece of code again in X weeks.

OTHER TIPS

IMHO most of your "pro" arguments are flawed, but let us discuss them one-by-one:

If the query needs to change and return a different type, we don't have to remember to rename the result variable.

If the query returns a different type in such a way that a name like accountDetails does not fit any more, the code depending on that variable will have to be changed, even if the variable name is just result. This is definitely easier (and thus less error prone) when the variable has a specific name. The only exception here is IMHO when there is no such code, like in a function where the result is just returned to the caller.

That also means code reviews won't be polluted with a bunch of files that have changed due to the renaming.

If you start arguing that way, you should never ever rename any variable in your code, regardless of how badly the name was chosen, just to make code reviews more effective. Maybe you should never do any refactorings? But then don't be astonished when your code quality degrades every day.

Moreover, in your example, result looks like a local variable, so the refactoring should typically not affect more than one file.

The job of a Web API controller isn't to know the details about what business logic is going on; it's just to invoke something and return it.

If var result = mediator.Send(query).Result; is a generic, reusable piece of code, processing different kinds queries and maybe returning different things, then this line is fine. However, if that would be your situation, then you probably would not have asked the question in the first place. I guess in this case, the query is always an accountFilter, and the result is always an accountDetails, and as long as that is true, your variable names should express that.

Of course, as I wrote above, if the next line in your function is just return result;, the unspecific name is ok.

If you really want to know what the line of code is doing, you can look at the context surrounding it.

Seriously? You want another maintenance programmer to decipher the surrounding context first to understand what is going on, whilst by simply picking a better name could save him half an hour of brain work? Be aware, in six months you might be the maintenance programmer for this line of code, and it will look to you as if someone else has written it first place. What would you prefer in that situation? A variable name telling you what it represents, or one for which you have to analyse the code first to be sure what it does?

So except for very simplistic cases where the whole function looks like

 AccountDetails GetAccountDetails(Query query)
 {
    var result = mediator.Send(query).Result;
    return result;
 }

you will probably be better off to use a more specific name (though - even for such a simple function - a parameter name like accountFilter instead of query will increase the readability heavily, and in this example, the result parameter is quite useless, I would typically prefer to write return mediator.Send(accountFilter).Result).

In general, you should prefer descriptive names. But, if the containing method/function is short (a couple of lines), well named, and if it's clear what is it that it returns, then naming the variable "result" is really not a problem.

However, as the method/function becomes bigger and more complex, using a non-descriptive, generic name, becomes more and more problematic.

The job of a Web API controller isn't to know the details about what business logic is going on; it's just to invoke something and return it.

Well... the name of the variable should indicate what the object/value obtain represents in the context of the surrounding code. If the name of the containing method is something like GetAccountDetails, then your argument doesn't really apply, and the name "accountDetails" is perfectly fine - but, if you still feel that the controller knows implementation details it shouldn't, then your abstractions in the bussiness logic layer could be a bit off.

Are vague names more maintainable?

No. It may be that there is no additional useful information to convey, but that calls for a generic name instead. Don't confuse everyone by not saying what you mean.

Are generic names more maintainable?

Depends. Is that generic name conveying everything important, or is there more useful information you could put into the name without getting too verbose or even repetitive?

Are specific names more maintainable?

Depends. Don't be overspecific or repetitive.

As always, context is king.

Playing devil's advocate to an extent, but lets consider the following code:

public List<Model> getModelsByTypeId(string modelTypeId)
{
    var result = new List<Model>();
    var dataset = myloverlysqlhelper(this.sqlForGettingModels, modelTypeId);
    //loop dataset and populate result...
    return result;
}

Here I have used 'generic' variable names instead of say modelResult and modelDataset and the meaning is made completely clear by the context.

I can copy and past the code into a dozen other similar functions and not have to edit the name in each.

But! even here I am still using modelTypeId instead of id or some more generic name. You could argue that the function name gives you enough context that id is justifiable, but would you ever put queryParameter? no, that wouldn't make any sense at all.

So, in summary, yes context allows you to be a little generic, you don't want to be having for(var integerWhichIAmCountingUpTo10 = 0;.... but generally a little extra description helps.

My answer would depend on the complexity of the code.

I've used result, but generally in contexts where the code is simple and/or the life of the variable is short.

If the code is complex (which it sounds like it's not in this case) vague variable names make it harder for the reader to understand the code because they have to waste cognitive overhead on remembering what the variable actually is.

Licensed under: CC-BY-SA with attribution
scroll top