I've seen braced initialization in a lot of contexts where the class of the object being initialized doesn't define an initializer_list constructor, so in that sense I think it's an idiomatic usage, and it has the advantage of disallowing narrowing implicit conversions like double to int. On the other hand, widespread use of braced initialization can cause some pain when you inadvertently use it for a type that does take an initializer_list when you meant to refer to a different constructor. I think the the latter case is worth avoiding, and in my opinion your proposal is in line with the spirit of the Mesos style guide as regards type deduction using auto, where it states:
The main goal is to increase code readability. This is safely the case if the exact same type omitted on the left is already fully stated on the right. In your example case with the braced initializer, the type isn't stated. I agree that the explicit construction should be required. On Mon, Feb 29, 2016 at 3:14 PM, Alexander Rojas <[email protected]> wrote: > Hi Guys, > > Today I was making a code review and I came across the following snippet: > > ``` > metrics.allocated.put( > name, > {strings::join("/", "allocator/allocated", name), > process::defer(self(), [this, name]() { > double result = 0; > foreachvalue (const Slave& slave, this->slaves) { > Option<Value::Scalar> value = > slave.allocated.get<Value::Scalar>(name); > if (value.isSome()) { > result += value.get().value(); > } > } > return result; > })}); > ``` > > I think by the code here is hard to tell what is happening here. If you > look at the declaration of `metrics.allocated` somewhere else you notice > that allocated has the following declaration: > > ``` > hashmap<std::string, process::metrics::Gauge> total; > ``` > > And gauge is certainly no container type, nor its constructor takes an > `std::initializer_list` as a parameter. In fact its signature is: > > ``` > Gauge::Gauge(const std::string& name, const Deferred<Future<double>()>& f) > ``` > > What is happening here is that brace constructors allows you to infer the > type being constructed, which makes the snippet there be equivalent to: > > ``` > metrics.allocated.put( > name, > Gauge( > strings::join("/", "allocator/allocated", name), > process::defer(self(), [this, name]() { > double result = 0; > foreachvalue (const Slave& slave, this->slaves) { > Option<Value::Scalar> value = > slave.allocated.get<Value::Scalar>(name); > if (value.isSome()) { > result += value.get().value(); > } > } > return result; > }))); > ``` > > What I’m proposing is to only allow this type of construction in a few > cases, namely for tuples, pairs and initializer lists where it actually > improves readability. > > Do you guys have any opinions on the matter?
