[
https://issues.apache.org/jira/browse/MESOS-2348?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14319380#comment-14319380
]
Michael Park edited comment on MESOS-2348 at 2/13/15 1:30 AM:
--------------------------------------------------------------
Hey Alex, thanks for the feedback!
I would agree with this approach if this statement were true: "a general filter
may be something more than just a map over resources with the given predicate".
A filter by definition is:
{quote}
a higher-order function that processes a data structure (typically a list) in
some order to produce a new data structure containing exactly those elements of
the original data structure for which a given predicate returns the boolean
value true.
{quote}
**Source**: http://en.wikipedia.org/wiki/Filter_%28higher-order_function%29
So what's going on with {{RoleFilter}}? This is a *great* example of how the
current design can lead people to believe that there are mysterious filters
that are somehow more generic than the "predicate filter". The {{RoleFilter}}
is actually 3 filters conflated into one, and simply dispatches to different
filters based on the given arguments.
Here is the mapping:
1. {{RoleFilter("R")}} => {{Resources::Filter::reserved("R")}}
2. {{RoleFilter("*")}} => {{Resources::Filter::unreserved()}}
3. {{RoleFilter()}} => {{Resources::Filter::any()}}
The current use of {{RoleFilter}} is when we want an {{initializer_list}} of
them.
{code}
std::initializer_list<RoleFilter> filters = {
RoleFilter(role),
RoleFilter("*"),
RoleFilter()
};
{code}
This is directly translated to:
{code}
std::initializer_list<Filter> filters = {
Resources::Filter::reserved(role),
Resources::Filter::unreserved(),
Resources::Filter::any()
};
{code}
If we actually wanted a unified interface for {{RoleFilter}}, we can also
provide that
{code}
class Resources {
public:
class Filter {
public:
static Filter any();
static Filter reserved(const std::string &role);
static Filter unreserved();
static Filter role() {
return any();
}
static Filter role(const std::string &role) {
return role == "*" ? unreserved() : reserved(role);
}
};
};
{code}
This is now the unified interface that dispatches to different filters:
{code}
std::initializer_list<Filter> filters = {
Resources::Filter::role(role),
Resources::Filter::role("*"),
Resources::Filter::role()
};
{code}
was (Author: mcypark):
Hey Alex, thanks for the feedback!
I would agree with this approach if this statement were true: "a general filter
may be something more than just a map over resources with the given predicate".
A filter by definition is:
{quote}
a higher-order function that processes a data structure (typically a list) in
some order to produce a new data structure containing exactly those elements of
the original data structure for which a given predicate returns the boolean
value true.
{quote}
**Source**: http://en.wikipedia.org/wiki/Filter_%28higher-order_function%29
So what's going on with {{RoleFilter}}? This is a **great** example of how the
current design can lead people to believe that there are mysterious filters
that are somehow more generic than the "predicate filter". The {{RoleFilter}}
is actually 3 filters conflated into one, and simply dispatches to different
filters based on the given arguments.
Here is the mapping:
1. {{RoleFilter("R")}} => {{Resources::Filter::reserved("R")}}
2. {{RoleFilter("*")}} => {{Resources::Filter::unreserved()}}
3. {{RoleFilter()}} => {{Resources::Filter::any()}}
The current use of {{RoleFilter}} is when we want an {{initializer_list}} of
them.
{code}
std::initializer_list<RoleFilter> filters = {
RoleFilter(role),
RoleFilter("*"),
RoleFilter()
};
{code}
This is directly translated to:
{code}
std::initializer_list<Filter> filters = {
Resources::Filter::reserved(role),
Resources::Filter::unreserved(),
Resources::Filter::any()
};
{code}
If we actually wanted a unified interface for {{RoleFilter}}, we can also
provide that
{code}
class Resources {
public:
class Filter {
public:
static Filter any();
static Filter reserved(const std::string &role);
static Filter unreserved();
static Filter role() {
return any();
}
static Filter role(const std::string &role) {
return role == "*" ? unreserved() : reserved(role);
}
};
};
{code}
This is now the unified interface that dispatches to different filters:
{code}
std::initializer_list<Filter> filters = {
Resources::Filter::role(role),
Resources::Filter::role("*"),
Resources::Filter::role()
};
{code}
> Introduce a new filter abstraction for Resources.
> -------------------------------------------------
>
> Key: MESOS-2348
> URL: https://issues.apache.org/jira/browse/MESOS-2348
> Project: Mesos
> Issue Type: Task
> Components: general
> Reporter: Michael Park
> Assignee: Michael Park
> Labels: mesosphere
>
> h1. Motivation
> The main motivation here is to improve the design of the filter abstraction
> for {{Resources}}. With the new design we gain:
> 1. No duplicate code.
> 2. No need to introduce a new class for every filter.
> 3. Safer code with less work.
> 4. Ability to compose filters.
> h2. Overview of the Current Design.
> I think I'll need to start here to provide a clear motivation. Here's the
> current design in short:
> {code}
> class Filter {
> public:
> virtual Resources apply(const Resources& resources) const = 0;
> };
> class FooFilter : public Filter {
> public:
> virtual Resources apply(const Resource& resources) const {
> Resources result;
> foreach (const Resource& resource, resources) {
> if (/* resource is Foo. */) {
> result += resource;
> }
> }
> return result;
> }
> };
> class BarFilter : public Filter {
> public:
> virtual Resources apply(const Resource& resources) const {
> Resources result;
> foreach (const Resource& resource, resources) {
> if (/* resource is Bar. */) {
> result += resource;
> }
> }
> return result;
> }
> };
> {code}
> h3. Disadvantages
> 1. Duplicate code.
> Every derived {{Filter}} will have duplicate code, in specific:
> {code}
> Resources result;
> foreach (const Resource& resource, resources) {
> if (/* resource satisfies some predicate. */) {
> result += resource;
> }
> }
> return result;
> {code}
> 2. Need to introduce a new class definition for every new {{Filter}}.
> We should be able to create new filters inline and use them in cases where
> the filter is only needed once and would only hurt readability at the global
> level. If the filter is useful in many contexts, by all means give it a name
> and put it somewhere.
> This is equivalent to lambda expressions which allow us to create new
> functions inline in cases where the function is only useful in this specific
> context but would only hurt readability at the global level. If the lambda is
> useful in many contexts, we give it a name and put it somewhere.
> 3. The constraints are too weak.
> A {{Filter}} must return a subset of the original {{Resources}}. It need not
> be a strict subset, but it must not be a strict superset. With the pure
> virtual apply, the only constraint we put on a new {{Filter}} definition is
> that we take {{Resources}} and return {{Resources}}. We should strive for
> code that prevents preventable bugs.
> 4. Inability to compose filters.
> I've defined 2 filters above, {{FooFilter}} and {{BarFilter}}. We should be
> able to give rise to a new filter with a composition of the filters above.
> For example, if I wanted to {{AND}} the filters, I shouldn't have to
> introduce {{FooAndBarFilter}}.
> This is equivalent to if we had predicates for these. Suppose we have
> {{isFoo(resource)}} and {{isBar(resource)}}. Would we introduce {{isNotFoo}},
> {{isNotBar}}, {{isFooAndBar}}, {{isFooOrBar}}, {{isFooNotBar}}, etc? If
> {{FooAndBar}} is a common concept we use all the time, sure. But in general,
> we would simply compose our predicates: {{!isFoo(resource)}},
> {{!isBar(resource)}}, {{isFoo(resource) && isBar(resource)}},
> {{isFoo(resource) || isBar(resource)}}, {{isFoo(resource) &&
> !isBar(resource)}}.
> h2. Overview of the New Design
> {code}
> class Filter {
> public:
> typedef lambda::function<bool(const Resource&)> Predicate;
> Filter(const Predicate& _predicate)
> : predicate(_predicate) {}
> Resources operator()(const Resources& resources) const {
> Resources result;
> foreach (const Resource& resource, resources) {
> if (predicate(resource)) {
> result += resource;
> }
> }
> return result;
> }
> Filter operator ! ();
> private:
> Filter operator && (
> const Filter &lhs,
> const Filter &rhs);
> Filter operator || (
> const Filter &lhs,
> const Filter &rhs);
> Predicate predicate;
> };
> bool isFoo(const Resource& resource) {
> return /* resource is Foo. */
> }
> Filter FooFilter = Filter(isFoo);
> Filter BarFilter = Filter([](const Resource& resource) {
> return /* resource is Bar. */
> });
> {code}
> h3. Addressing the Disadvantages
> 1. No duplicate code.
> We've removed the duplicate code by making the predicate the customization
> point.
> 2. No need to introduce a new class definition.
> We can certainly introduce named filters if they're useful in many contexts
> such as {{FooFilter}} and {{BarFilter}} above, but there's no *need* to.
> 3. The constraints are strong.
> The creator of a new {{Filter}} simply specifies the predicate. The
> constraint, "filter returns a subset of the original Resources" cannot be
> violated.
> 4. Composability.
> Using the overloaded logical operators (We can change this to be named
> functions if people prefer), we can compose existing filters.
> {{Filter FooAndBarFilter = FooFilter && BarFilter;}} gives us a composed
> filter that requires both filters to be satisfied.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
