----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46814/#review131168 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 710 - 725) <https://reviews.apache.org/r/46814/#comment195114> How about: ```cpp bool is_negated = strings::startsWith(name, "no-"); std::string flag_name = !is_negated ? name : name.substr(3)s auto iter = flags_.find(flag_name); if (iter == flags_.end()) { if (!unknowns) { return Error("Failed to load unknown flag '" + flag_name + "'" + (!is_negated ? "" : " via '" + name + "'")); } else { continue; } } ``` 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 715 - 720) <https://reviews.apache.org/r/46814/#comment195093> This block is indented 3 spaces. 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 727 - 764) <https://reviews.apache.org/r/46814/#comment195116> How about splitting the conditions first by non-boolean vs boolean flags? I think this way we can group the logical parts together better. Consider the positions of "true" and "false", for example. ```cpp const Flag& flag = iter->second; std::string value_; if (!flag.boolean) { // non-boolean flag if (is_negated) { return Error("Failed to load non-boolean flag '" + flag_name + "' via '" + name + "'"); } if (value.isNone()) { return Error("Failed to load non-boolean flag '" + flag_name + "': Missing value"); } value_ = value.get(); } else { // boolean flag if (value.isNone() || value.get() == "") { value_ = !is_negated ? "true" : "false"; } else if (!is_negated) { value_ = value.get(); } else { return Error( "Failed to load boolean flag '" + flag_name + "' via '" + name + "' with value '" + value.get() + "'"); } } Try<Nothing> load = flag.load(this, value_); if (load.isError()) { return Error("Failed to load flag '" + flag_name + "': " + load.error()); } ``` - Michael Park On April 29, 2016, 6:43 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46814/ > ----------------------------------------------------------- > > (Updated April 29, 2016, 6:43 a.m.) > > > Review request for mesos, Ben Mahler, Greg Mann, and Michael Park. > > > Bugs: MESOS-5271 > https://issues.apache.org/jira/browse/MESOS-5271 > > > Repository: mesos > > > Description > ------- > > Made this function more readable. The error strings are still kept the > same as the previous code. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp > c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 > > Diff: https://reviews.apache.org/r/46814/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
