> 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?
>
> Ben Mahler wrote:
> 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").
Yup, I think this is more philosophical than technical. For me the important
thing is that a function has to do one thing and is easy to understand based on
its signature. And yes, I don't like reading functions that have 100s of lines.
I tend to get lost trying to understand what is happening.
- Vinod
-----------------------------------------------------------
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
>
>
