Replace traditional for with ranged for in block and transaction primitives#10892
Replace traditional for with ranged for in block and transaction primitives#10892laanwj merged 1 commit intobitcoin:masterfrom bytting:20170721-rangedfor-primitives
Conversation
src/primitives/block.cpp
Outdated
| for (unsigned int i = 0; i < vtx.size(); i++) | ||
| { | ||
| s << " " << vtx[i]->ToString() << "\n"; | ||
| for (const auto &tx : vtx) { |
There was a problem hiding this comment.
Nit: const auto& tx seems to be the normal way to format it.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 
I guess it's not that bad for arguments and return types though, I'll look into it.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
for (const CKeyID &keyid : GetKeys()) {
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Okay, your argument that a reference is a distinct type is consistent with the C++ standard, so I give up. GJ
|
utACK 72f0060 |
|
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.
|
|
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 |
|
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. |
|
utACK |
…ansaction primitives 72f0060 Replace traditional for with ranged for in primitives (Dag Robole) Pull request description: Replace traditional for with ranged for in block and transaction primitives to improve readability Tree-SHA512: c0fff603d2939149ca48b6aa72b59738a3658d49bd58b2d4ffbc85bdb774d8d5bb808fe526fe22bb9eb214de632834d373e2aab44f6019a83c0b09440cea6528
… and transaction primitives 72f0060 Replace traditional for with ranged for in primitives (Dag Robole) Pull request description: Replace traditional for with ranged for in block and transaction primitives to improve readability Tree-SHA512: c0fff603d2939149ca48b6aa72b59738a3658d49bd58b2d4ffbc85bdb774d8d5bb808fe526fe22bb9eb214de632834d373e2aab44f6019a83c0b09440cea6528
7 participants
Replace traditional for with ranged for in block and transaction primitives to improve readability