Good practice to explicitly declare variables without 'much' use in favor of readability?

Question

So, this is yet another 'good' programming practice question. I did search around a bit, but something like this is often hard to define in just a few words.

To the question: from a professional perspective, is it better programming practice to keep code streamlined and short (not necessarily more efficient) or to explicitly define instance variables only to assign them and immediately return? For example:

FILE * foo(){
    FILE * retVal; 
    foo2(); 
    retVal = foobar(); 
    return retVal;
}

From the above, we can see immediately that foobar returns a FILE *. So, from this style of programming we can more quickly extract important information. This is true compared to something like this:

FILE * foo(){
    foo2(); 
    return foobar(); 
}

Which, of course, accomplishes the same thing. However, one must look more deeply to find the same information. I tend to favor the latter style of programming simply for the reason that it looks better. Due to the nature of how this program will run, I doubt there are any immediate performance gains from using either one since memory is still necessary for either choice - the difference is whether or not the user or the compiler allocates it.

As another example of keeping code short and concise:

int foo(){
    int i = 0;     
    while(foobar())
        i++:    
    return i;
}

TL:DR Question >> Is it better to explicitly show what is being done, or is it okay, in favour of brevity and conciseness, shorten code that accomplishes the same task but does not necessarily provide performance gain?

Answer 1

Short version

Only introduce new variable when they add non obvious informations. Usually that's the case when the variable is used to replace a somewhat complex expression

Long version

As in everything, it depends. I think the best way of approaching this is by doing a cost benefit analysis of the situation.

The cost of using an intermediate variable (purely from code quality/understanding point of view, i am pretty sure any decently modern compiler will optimize those away) is i would say the effort of parsing the variable, understand the definition in context, and more importantly relate the latter use of that variable to the working model of program that whomever is reading your code has in his mind.

The benefit is that a new element, by introducing more information, can help the reader to either form a simpler mental model of the code base, or a more precise model. And the for a new variable declaration, most information are contained in either the type or the name.

Consider for example the two following example:

if(isSocial)
     return map[*std::min(d.begin(),d.end())].first;
else
     return map[*std::max(d.begin(),d.end())].first;
return idealAge;


if(isSocial)
     int closestPersonAge = map[*std::min(d.begin(),d.end())].first;
     idealAge = closestPersonAge
else
     int futhestPersonAge = map[*std::max(d.begin(),d.end())].first;
     idealAge = futhestPersonAge
return idealAge;

In the first example your reader would need to understand what std::min does, what are 'd' and 'map', what are their type, what the type of the element of map etc... In the second example by providing meaningful variable name, you essentially save the reader from having to understand the computation , thus allowing him to have a simpler mental model of the code while roughly keeping the same amount of important information.

Now compare that to :

int personAge = person.age();
return personAge;

In this case, i think having personAge doesnt add any meanifull information (the variable and method name convey the same information), and thus is not really helping the reader in any way.

Answer 2

Disclaimer: Nothing written below is from any standard.

Usually, with proper optimization turned on, compilers will optimize out most of the redundant or dead part and make the binary as efficient as possible.

Keeping that in mind, it is advised that to write code which is easily understandable for humans. Leave the optimization part (mostly) to the compiler.

Writing a code which is more understandable by humans, makes it

• More acceptable to others
• Easier to maintain
• Easier to debug
• Last but not the least, A lifesaver for you ^{(PUN FUN intended)}

Answer 3

There is readability, and there is debugability.

I would have written your example (which doesn't compile, by the way) as

FILE* foo ()
{
    foo2(); 
    FILE* retVal = foobar(); 
    return retVal;
}

That way, if I need to debug, I can set a breakpoint on the return statement and see what retVal is. It's also usually a good idea to avoid overly complicated expressions and use intermediate variables. First, for easier debugging, and second, for easier reading.

Answer 4

The choice between accurate and shortened code is subjective due to the reason(s) you are projecting for. When it comes to maintenance the majority of us would prefer brief code. Even learners would prefer brief code despite the fact this is the contrary of what they must prefer.

C is designed to be human-readable and to be compiled with less efforts as possible. It is procedural and very ornate-less. One more reason to code in favor of readability and against time consumption.

---

Both the ways you provided in the example generate exactly the same ASM code (note the -O).

            .Ltext0:
                    .globl  foobar
                foobar:
                .LFB13:
                    .cfi_startproc
0000 B8000000       movl    $0, %eax
     00
0005 C3             ret
                    .cfi_endproc
                .LFE13:
                    .section    .rodata.str1.1,"aMS",@progbits,1
                .LC0:
0000 666F6F32       .string "foo2 called"
     2063616C 
     6C656400 
                    .text
                    .globl  foo2
                foo2:
                .LFB14:
                    .cfi_startproc
0006 4883EC08       subq    $8, %rsp
                    .cfi_def_cfa_offset 16
000a BF000000       movl    $.LC0, %edi
     00
000f E8000000       call    puts
     00
                .LVL0:
0014 B8000000       movl    $0, %eax
     00
0019 4883C408       addq    $8, %rsp
                    .cfi_def_cfa_offset 8
001d C3             ret
                    .cfi_endproc
                .LFE14:
                    .globl  foo
                foo:
                .LFB15:
                    .cfi_startproc
001e 4883EC08       subq    $8, %rsp
                    .cfi_def_cfa_offset 16
0022 B8000000       movl    $0, %eax
     00
0027 E8000000       call    foo2
     00
                .LVL1:
002c B8000000       movl    $0, %eax
     00
0031 4883C408       addq    $8, %rsp
                    .cfi_def_cfa_offset 8
0035 C3             ret
                    .cfi_endproc
                .LFE15:
                    .globl  main
                main:
                .LFB16:
                    .cfi_startproc
0036 4883EC08       subq    $8, %rsp
                    .cfi_def_cfa_offset 16
                .LBB8:
                .LBB9:
003a B8000000       movl    $0, %eax
     00
003f E8000000       call    foo2
     00
                .LVL2:
                .LBE9:
                .LBE8:
0044 B8000000       movl    $0, %eax
     00
0049 4883C408       addq    $8, %rsp
                    .cfi_def_cfa_offset 8
004d C3             ret
                    .cfi_endproc
                .LFE16:
                .Letext0:

..in a response of your minimalistic, insignificant brief way and terse way.

---

Taking this in consideration I can freely say that it is best if you simply apply both correctly. And that is.. brief and clear as possible and

/* COMMENTED */

Answer 5

I'm opposed to this kind of style, although I understand that many do it for debugging purposes.

It can be seen as a fallacy in typical debuggers that it can be hard to see values that are immediately returned, perhaps.

If you feel you want to do this, I recommend two things very very highly:

• Use C99, so you can declare things late.
• Use const.

So I would write the foo() example as this, if I had to:

FILE * foo(void)
{
  foo2();
  FILE * const retVal = foobar();
  return retVal;
}

Note that the const can't go to the left of the asterisk (const FILE *retVal = ...) since that would make the type const FILE *; what we want is a constant pointer, not a pointer to something that is constant.

The purpose of the constness is to say to human readers "I'm naming this value here, but this isn't state that I'm going to be mucking about with".

Answer 6

I agree with code to be readable. However, I disagree the first one is easier to read or even maintain.

• readbility: More code to read and understand. While for the example this might not be that hard, it might for more complex types.
• maintainablity: If you change the return type, you will have to change the retval declaratation, too.

Many coding styles require variables defined at the beginning of a block. Some of them even only allow that at the function-level, beginning. So you have the variable declared near the function declaration, far away from the return.

Even if that is allowed: what have you gained? It might hide coercions on return as well, as the compiler will complain also about wrong return type - if you enable most warnings. This will add to code quality (if taken seriously).