Replace traditional for with ranged for in block and transaction primitives #10892

pull bytting wants to merge 1 commits into bitcoin:master from bytting:20170721-rangedfor-primitives changing 2 files +11 −13

bytting commented at 10:33 AM on July 21, 2017: contributor

Replace traditional for with ranged for in block and transaction primitives to improve readability
jonasschnelli added the label Refactoring on Jul 21, 2017
dcousens approved
Replace traditional for with ranged for in primitives 72f00608d0
in src/primitives/block.cpp:28 in 020d71d46f outdated
```
  24 | @@ -25,9 +25,8 @@ std::string CBlock::ToString() const
  25 |          hashMerkleRoot.ToString(),
  26 |          nTime, nBits, nNonce,
  27 |          vtx.size());
  28 | -    for (unsigned int i = 0; i < vtx.size(); i++)
  29 | -    {
  30 | -        s << "  " << vtx[i]->ToString() << "\n";
  31 | +    for (const auto &tx : vtx) {
```
benma commented at 5:50 AM on July 24, 2017:

Nit: const auto& tx seems to be the normal way to format it.
bytting commented at 7:13 AM on July 24, 2017:

Yeah, I was unsure about that. It seems counter intuitive to me since it is the variable that is a reference, not the type.

Consider this case:

int *a, *b, &c = value;

I have seen both being used in this code base, so I think I'll leave it unless there is more protest.

Thanks though
benma commented at 9:03 AM on July 24, 2017:

I did check, auto& is much more common in the codebase (60 times vs. sth. like 5 times).

It seems counter intuitive to me since it is the variable that is a reference, not the type.

It's actually the type. It's apparent if you look at function signatures.

void foo(const std::string&); is a valid function signature (first argument is a reference type).

bytting commented at 10:17 AM on July 24, 2017:

Hm, when I look at your last patch #10916 you actually do the same.

first argument is a reference type

I would call that an unnamed reference to type std::string :trollface:

I guess it's not that bad for arguments and return types though, I'll look into it.

benma commented at 10:21 AM on July 24, 2017:

Hm, when I look at your last patch #10916 you actually do the same.

You mean using const auto& var, not const auto &var? Makes sense that I use what I advocate :)

bytting commented at 10:23 AM on July 24, 2017:

for (const CKeyID &keyid : GetKeys()) {

benma commented at 10:26 AM on July 24, 2017:

Good catch! In my defense, that line was there like that before and is one of the 5 or so instances like that I refered to earlier. I'll fix it in a few hours when I am back home.

bytting commented at 10:29 AM on July 24, 2017:

Okay, your argument that a reference is a distinct type is consistent with the C++ standard, so I give up. GJ
benma commented at 8:21 PM on July 24, 2017: none

utACK 72f00608d0a130a1488dc49df1073abe01d49519
sipa commented at 8:31 PM on July 24, 2017: member

In C++ syntax, elements like [], *, and & belong with the variable, not the type. An example is int *a, b, which declares b as an int, not as a pointer to int.

However, in terms of style recommendation, this is controversial, and the codebase contains plenty of examples of left and right 'hugging'. I wouldn't worry about using one over the other.
benma commented at 8:46 PM on July 24, 2017: none

I guess you're right that it doesn't matter, I just thought it made sense to stick with the prelevant style in the code. Personally I put it with the type because of things like std::vector<Foo*> and func signatures.
ryanofsky commented at 9:14 PM on July 24, 2017: member

Currently we have PointerAlignment: Left in our clang-format file. This puts pointers and reference symbols next to the type instead of the variable. Should probably remove the setting or change it if this style isn't actually preferred.
sipa commented at 9:45 PM on July 24, 2017: member

utACK
laanwj merged this on Jul 27, 2017
laanwj closed this on Jul 27, 2017
laanwj referenced this in commit ba1bbb049b on Jul 27, 2017
PastaPastaPasta referenced this in commit 80bc6df26f on Aug 2, 2019
PastaPastaPasta referenced this in commit c35bdf89f4 on Aug 6, 2019
barrystyle referenced this in commit 7bd753311d on Jan 22, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels