mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Park" <mcyp...@gmail.com>
Subject Re: Review Request 30395: etcd master contender + detector
Date Thu, 29 Jan 2015 21:06:26 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message