> On May 14, 2014, 12:13 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 3670-3696 > > <https://reviews.apache.org/r/19504/diff/17/?file=581329#file581329line3670> > > > > Wouldn't the following be simpler and easier to understand for those > > reading these functions? > > > > double Master::_active_slaves() > > { > > double count = 0.0; > > foreachvalue (Slave* slave, slaves.activated) { > > if (!slave->disconnected) { > > count++; > > } > > } > > return count; > > } > > > > > > double Master::_inactive_slaves() > > { > > double count = 0.0; > > foreachvalue (Slave* slave, slaves.activated) { > > if (slave->disconnected) { > > count++; > > } > > } > > return count; > > } > > > > It seems odd that the forward declaration of 'struct Slave' was not > > sufficient to place these in the header, since Slave was also defined in > > the header further down, ah well.
Thanks! I have always preferred to use the standard algorithms wherever possible as they may have optimal implementations for some container types (not in this case, probably). I find them quite readable but I do like the explicit loop too. The forward declaration is just that; it tells the compiler "There's a symbol named Slave". To compile these method definitions the compiler would need to know the symbol named Slave has a field named 'disconnected' that is convertible to bool. For that, it needs the full definition to be available. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19504/#review43001 ----------------------------------------------------------- On May 13, 2014, 9:53 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19504/ > ----------------------------------------------------------- > > (Updated May 13, 2014, 9:53 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1132 and MESOS-1332 > https://issues.apache.org/jira/browse/MESOS-1132 > https://issues.apache.org/jira/browse/MESOS-1332 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > src/master/http.cpp f54c841d003d7a4d2eda5e93405afa358707f4e0 > src/master/master.hpp 4f9ae36c822a16ea3baadf6b9fa3616d030d19f2 > src/master/master.cpp d5453673a839326d00a3d45940bd4562c526cff2 > src/tests/master_tests.cpp 7aa678afc94869c8243485bd0604532dec43a1e2 > > Diff: https://reviews.apache.org/r/19504/diff/ > > > Testing > ------- > > ran locally and checked /metrics/snapshot vs master/stats.json > > make check > > > Thanks, > > Dominic Hamon > >
