> On Jan. 29, 2015, 5:50 p.m., Alexander Rukletsov wrote:
> > src/master/contender.cpp, line 82
> > <https://reviews.apache.org/r/30395/diff/1/?file=839643#file839643line82>
> >
> >     You can safely omit trailing underscores.

In this case, yes that is true. This topic has come up before and perhaps it's 
better suited for the dev list so that we can get a community-wide consensus on 
this.

In general, I think we should try to avoid shadowing. *Especially* between 
local variables (including function parameters) and member variables because 
the member variable declarations are typically lexically not in scope.

1)

```
struct Obj
{
  // Original author: "Eh, we don't need to refer to the member 'foo' so this 
is fine."
  Obj(Foo &&foo)
      : foo(std::move(foo)) {}
  
  Foo foo;
};
```

```
struct Obj
{
  Obj(Foo &&foo)
      : foo(std::move(foo)) {
    // New author: "ok, let's do some stuff with member 'foo'!"
    // Oops... this will segfault.
    std::cout << foo << std::endl;
  }
  
  Foo foo;
};
```

2)

```
// Original author: "There's a member 'foo' but I don't need it for this 
function."
Obj::F(const Foo &foo) {
  // Do stuff with local 'foo' ...
}
```

```
Obj::F(const Foo &foo) {
  // Do stuff with local 'foo' ...
  // /* ... */
  // New author: (middle of debugging) "What state is (member) 'foo' in...?"
  // Local 'foo' gets printed instead, doesn't realize the problem until they 
look above
  // to find the shadowed variable and... fury.
  std::cout << foo << std::endl;
}
```

In short, I think we should opt to stay away from running into issues like this 
by not shadowing member variables.
It keeps us away from running into stupid bugs that waste developer's time, and 
also eliminates the need to reevaluate if the names of variables need to be 
changed when adding new code.


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30395/#review70231
-----------------------------------------------------------


On Jan. 29, 2015, 3:58 a.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30395/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-1806
>     https://issues.apache.org/jira/browse/MESOS-1806
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> etcd master contender + detector
> 
> 
> Diffs
> -----
> 
>   src/etcd/etcd.cpp PRE-CREATION 
>   src/master/contender.hpp 76beb5f973ae02507849233b6d73c43293669489 
>   src/master/contender.cpp c1bf82b621d6b46afe001acafe9ee53336726406 
>   src/master/detector.hpp 2905e2b3536e14e9df3570da172603e6ed81aae1 
>   src/master/detector.cpp 700eb9dde8e71648bacc00a82766634f77cf2d15 
> 
> Diff: https://reviews.apache.org/r/30395/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Reply via email to