Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##First Pass:

First Pass:

##First Pass:

First Pass:

added 9 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
-Wall -Wextra -Wshadow -Werror
-Wall -Wextra -Werror
-Wall -Wextra -Wshadow -Werror
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

##First Pass:

void Board::traverse (Tile *t, Neighbours neighbours)
 ^^^^ Pointers are bad idea.
 They make resource management hard to 
 reason about as pointers have no
 ownership semantics. Prefer a reference
 if it can not be nullptr.
void Board::traverse (Tile *t, Neighbours neighbours)
 ^^^^^^^^^^^^^^^^^^^^^ 
 Here you are passing neighbours by value.
 This means you are making a copy of the
 object. Prefer const reference.
 // What is the type of COO
 // is there a better way to create it?
 COO coo = make_pair(t->i, t->j);
 // Here you are copy the content of `neighbours[coo]`
 // into buddies. Do you need a copy or can we just
 // use a reference to the original value.
 Buddies buddies = neighbours[coo];
 // Prefer a reference to a pointer.
 Tile *buddy = &this->field[buddies[i]];
 // Here you are making a copy of buddy.
 // Do you really need another copy?
 this->chains[buddy->chainId].push_back(*buddy);
 // Again you are copy the value of tile into chains.
 this->chains[t->chainId].push_back(*t);
 // Another copy of tiles is being put here.
 this->chains[t->chainId].push_back(*t);
 // And you are copying neighbours into the
 // recursive call here.
 this->traverse(&this->field[buddies[i]], neighbours);

Use of this

Your use of this is not idiomatic.

this->chains[t->chainId].push_back(*t);

Its much easier to write:

chains[t->chainId].push_back(*t);

The only reason to use this is to disambiguate the use of shadowed variables. But shadowed variables is an error waiting to happen. So it is best to avoid shadowed variables entirely (even turn on your errors to make sure they don't happen)

-Wall -Wextra -Werror
default

AltStyle によって変換されたページ (->オリジナル) /