0

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.
Improve this question
7
  • 6
    Code is more often read than written. So make it as readable as you can. – user2180613 Apr 23 '18 at 14:01
  • 6
    The single bullet point in #2 is more important than everything in #1 combined. – 17 of 26 Apr 23 '18 at 16:59
  • 2
    If vague names truly were better, wouldn't we name everything "a", "it", or "thingie"? – user949300 Apr 23 '18 at 18:25
  • 1
    "If the query needs to change and return a different type, we don't have to remember to rename the result variable." <- Most people put bread in their toaster and make toast. I put input in my appliance and get output. That way I don't have to remember to rename it if I replace my toaster with a waffle iron. – user253751 Apr 24 '18 at 11:53
  • 1
    Let me give you an example of how much more maintainable they are... we had to adjust a few things in an old program that was written about five years ago or so (by someone who already left the company). Apparently he had the same idea as yours about variable naming, everything was generic. So we spent nearly an entire week fixing their names and trying to crack how this damn thing worked. We managed to make it pretty and understandable for anyone at last but wasted a week on doing only that and haven't done any adjustments yet. – t3chb0t Apr 26 '18 at 8:17
18

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.

Improve this answer
10
  • 2
    So if you use the IDE to hover over the variable to see what it is, that means you wanted to know what it was. So why not name it something explicit so you don't even need to hover? Isn't it appropriate for code to read like a book, where you don't need to pause and figure stuff out along the way? – Bob Horn Apr 23 '18 at 16:43
  • 1
    @BobHorn yes it is just like a book, with footnotes or further reading. Most of the time, the prose is enough. Need more details? Refer to the footnotes. Likewise var should be adequate for most of the time. Occasionally you’ll need the extra detail and then the hover “footnotes” comes into play. That way the code isn’t cluttered with more detail than is needed for most of the time. – David Arno Apr 23 '18 at 21:07
  • 4
    When I was browsing code diffs in git five minutes ago – axl Apr 24 '18 at 5:14
  • 1
    @Bob Horn I like the book & footnote example. The naming should be precise enough to give the read a good idea what is going on (without needing to reference 20+ pages), but including footnotes (i.e. the IDE's tooltip) to give additional details if so desired (without directly adding to the current chapter a George R.R. Martin-esque 5-page description of the food-spread on the table) – CharonX Apr 24 '18 at 9:35
  • 1
    If the code is written well, you should rarely care about the actual variable type. The compiler will yell at you if you screw up your types. As a developer, focus on getting your business logic right. That means things like 'WeeklyEarnings = HourlyWage * HoursWorked'. Most of the time, you don't care if those are all int, double, or float. – 17 of 26 Apr 24 '18 at 16:03
12

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).

Improve this answer
4
  • In your last example, why even use vague variable names there? If using explicit variable names is less cognitive overhead in complex methods, isn't it also less cognitive overhead in simple methods as well? – Bob Horn Apr 23 '18 at 16:40
  • 1
    @BobHorn: sure, see my edit. – Doc Brown Apr 23 '18 at 17:22
  • I struggled between picking this answer and the other answer. Thank you for your input. I loved this line: "Seriously? You want another maintenance programmer to decipher the surrounding context first to understand what is going on?" – Bob Horn Apr 23 '18 at 20:25
  • 1
    Following OP's naming convention the method should be called GetResult... oh dear... good luck trying to understand the context. – t3chb0t Apr 24 '18 at 6:11
3

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.

Improve this answer
3

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.

Improve this answer
1

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.

Improve this answer
6
  • 3
    Copy and paste? You made the code general via the names and design. Go all out and make it completely reusable via generics and/or delegates. I imagine you would do this, but you did say "copy and paste". – Frank Hileman Apr 23 '18 at 16:24
  • generic repos are bad – Ewan Apr 23 '18 at 16:30
  • I would say all repositories are bad, but it was about variables. Your point in the answer is correct, I just would not promote copy and paste. – Frank Hileman Apr 23 '18 at 19:02
  • 1
    copy and paste with minor changes is sometimes better than making things super generic – Ewan Apr 23 '18 at 19:09
  • 1
    I guess the point is that, the one-sentence mentioning of "copy and past (sic)" seems to distract (or even ruin) an otherwise good answer. I would rephrase your answer in this way: for small functions, the function name already provides some context; function arguments can use a little bit of extra naming juice (for context) if that helps further reduce confusion, or to promote consistency (uniformity, pattern) in the user's brain. In a small function like this, bikeshedding about local variable name is nobody's business. Functions longer than 10 lines of code need written more carefully. – rwong Apr 24 '18 at 0:19
0

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.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.