13

In C++, is it bad practice create blocks of code inside some function, such as the following:

    bool f()
    {
       {
           double test = 0;

       test = // some other variable outside this function, for example.

       if (test == // some value)
         return true;
   }

   {
       double test = 0;

       test = // some variable outside this function, different from the last one.

       if (test == // some value)
         return true;
   }

   return false;

}

The point of doing this would be to use the same variable name of "test" multiple times, for the same type of procedure. In my actual project, I have multiple variables and am performing multiple tests. I don't really want to keep creating new variables with different names for each of the tests, considering how the tests are so similar.

Is it bad practice to insert blocks of code so that the variables go out of scope after each test, and then I can use their names again? Or should I seek another solution? It should be noted that I considered using the same set of variables for all my tests (and just setting them all to 0 after each test was over), but I was under the impression this may be bad practice.

Inertial Ignorance
  • 565
  • 27
    If I were reveiwing this code, I would tell you to separate each of these tests into individual methods... As a consequence you wouldn't need to use code blocks to scope them seperately. – Maybe_Factor Nov 02 '17 at 04:34
  • 1
    @Maybe_Factor - I agree. Benefit of separate methods is that you can name each block, providing a more readable code. – mouviciel Nov 02 '17 at 08:54
  • 1
    @mouviciel Not just more readable code, but more readable test reports! – Maybe_Factor Nov 02 '17 at 23:01
  • 1
    @Maybe_Factor I disagree. Moving things into separate functions has the negative effect of making them seem like little reusable bits of functionality. Keeping all the logic for a function in one place is good. – mrr Nov 07 '17 at 22:19
  • 2
    @MilesRout This isn't one logical function, this is multiple unit tests for a function all crammed into a single test function. Code blocks vs functions in normal code is another discussion entirely. – Maybe_Factor Nov 08 '17 at 05:15
  • @Maybe_Factor Ah you're right... not sure how I missed that the first time – mrr Nov 08 '17 at 22:47
  • 1
    One can think of reasons to solve the multiple-identifier-name-for-the-same-thing problem alternatively. But if you have multiple steps of a semantically single operation that mean nothing outside this particular method, I say this is better than using a shared variable that is declared all the way up or different methods, because this keeps your scope tighter. And that is a good thing. – Martin Maat Jun 15 '21 at 14:21

2 Answers2

23

Blocks are perfectly reasonable if you're using them to scope some resource. Files, network connections, memory allocations, database transactions, whatever. In those cases, the block is actually part of the logical structure of the code: you spawn a resource, it exists for some period of time, and then it goes away at a designated time.

But if all you're doing is scoping a name, then I would say that they are bad practice. Generally speaking, of course; special circumstances can apply.

For example, if this function were generated by some code generation system, testing framework, or the like, then blocks for the sake of name scoping is a reasonable thing. But you'd be talking about code written for the purpose of a machine, not a human.

If a human is writing code where they need to reuse names within the same function, I would say that those blocks probably need to be separate functions. Especially if those names are being used with different types and/or meaning within those blocks.

Nicol Bolas
  • 11,913
  • 4
  • 37
  • 46
  • If you do introduce a “useless” block for RAII reasons, it's a good idea to comment it (// Limit scope of DBConnection object, because it needs to be closed before doing Task X). – dan04 Jun 16 '21 at 21:32
  • @dan04: I feel like if you have to leave a comment like that, it suggests that the people reading your code aren't expected to know how RAII works or what DBConnection is supposed to do. – Nicol Bolas Jun 16 '21 at 21:38
11

It is not bad practice to create blocks like this. It's how scopes work.

Usually this is done when using RAII (Resource Acquisition is Initialization) and you want to control when the destructors get called.

If it gets long, I would consider moving that code to it's own function.

In my opinion just using it to recycle variable names is not a good idea. But I can see it been useful in cases with low memory

Robert Andrzejuk
  • 590