> On June 20, 2013, 11:16 p.m., Ben Mahler wrote:
> > I see why you added the helper, but it seems odd that it's only needed /
> > used by initialization. I realize initialize is fairly large, but my
> > preference would be to keep it in there. Alternatively, could the helper be
> > static, taking as input the flags, and returning a Resources object?
>
> Vinod Kone wrote:
> I really don't want to put it up in initialize(), because as you noted,
> its ~200 lines already! I think it reads pretty clear when you see it in
> initialize(). Infact, I would even like all the installs to be refactored
> into something like setupRoutes().
>
> Regarding a static helper, I don't understand the reasoning?
I guess this boils down to philosophy, I find it easier to read a long version
of initialize() instead of jumping into helpers. With these helpers I would say
they are especially confusing because they do not reduce complexity, they
merely pull code from a function into a sub-function (but that anyone can
call!). Ultimately, this actually increases complexity of the code (same code,
but more lines to express it, and I have to jump into different contexts to
understand initialize()).
The static helper would at least have the benefit of not modifying state in the
slave, meaning one can clearly see how it is used ("given a Flags object, this
produces a Resources object and has no effect on the Slave because it's a
static function").
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12010/#review22208
-----------------------------------------------------------
On June 20, 2013, 11:02 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12010/
> -----------------------------------------------------------
>
> (Updated June 20, 2013, 11:02 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Jiang Yan Xu.
>
>
> Description
> -------
>
> See summary
>
>
> Diffs
> -----
>
> src/slave/slave.hpp d1ba82ed0c87331c6c06b109bbc2b267a07cf335
> src/slave/slave.cpp 952bd149247c2cc44b87d20fe63a45d011f5578b
>
> Diff: https://reviews.apache.org/r/12010/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
