> On June 10, 2015, 3:56 p.m., Vinod Kone wrote: > > src/common/resources.cpp, lines 383-388 > > <https://reviews.apache.org/r/35327/diff/1/?file=982274#file982274line383> > > > > No need for else if. > > > > if () { > > > > } > > > > nameTypes[name] = resource.get().type(); > > Jiang Yan Xu wrote: > Just that the if ... else ... structure logically separates the chunk of > code that does the bookkeeping from the rest of the sequence of logic. > > e.g. > ```cpp > if () { > } > > nameTypes[name] = resource.get().type(); > > resources += resource.get(); > ``` > > I could to remove the blank line between the if block and the subsquence > line like this: > > ```cpp > if () { > } > nameTypes[name] = resource.get().type(); > ``` > > But we generally don't do this, I think.
Oh I didn't even realize that it's an "else if" rather than "else". In this case `nameTypes[name] = resource.get().type();` is a (logical) noop if `nameTypes.contains(name)` but anyways, I think in this case leaving the `else` there is fine. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35327/#review87482 ----------------------------------------------------------- On June 10, 2015, 3:34 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35327/ > ----------------------------------------------------------- > > (Updated June 10, 2015, 3:34 p.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2854 > https://issues.apache.org/jira/browse/MESOS-2854 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/common/resources.cpp c168bd83d0e0f07fb8f3a70114dd854cad5f5140 > src/tests/resources_tests.cpp b1e4483695eda998129db2c89a9dce044607c8b0 > > Diff: https://reviews.apache.org/r/35327/diff/ > > > Testing > ------- > > make check. > > Updated test `ResourcesTest.ParseError` to cover this case. > > > Thanks, > > Jiang Yan Xu > >
