[ 
https://issues.apache.org/jira/browse/MESOS-1064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14251424#comment-14251424
 ] 

Michael Park commented on MESOS-1064:
-------------------------------------

Enabling unit-testing here would be great and yes, we should make validators 
only take the parameters it cares about!

My thought here is that we should keep things as they are, "bind" the arguments 
for a validator at construction time and make the `virtual operator()` be 
nullary.

Here's a sketch of it.

{code}
class Validator {
  public:

  virtual Option<Error> operator () () const = 0;

  virtual ~Validator() = default;

};

class IdValidator : public Validator {
  public:

  IdValidator(std::string id_) : id(std::move(id_)) {}

  virtual Option<Error> operator () () const override {
    // Validate id.
  };

  private:

  std::string id;

};

class CheckpointValidator : public Validator {
  public:

  CheckpointValidator(std::string framework_, std::string slave_)
    : framework(std::move(framework_)), slave(std::move(slave_)) {}

  virtual Option<Error> operator () () const override {
    // Validate framework, slave.
  }

  private:

  std::string framework;

  std::string slave;

};

Option<Error> validateTask(
    const std::string &id,
    const std::string &framework,
    const std::string &slave) {
  std::initializer_list<std::unique_ptr<Validator>> validators = {
    std::make_unique<IdValidator>(id),
    std::make_unique<CheckpointValidator>(framework, slave)
  };
  for (const std::unique_ptr<Validator>& validator : validators) {
    Option<Error> error = (*validator)();
    if (error.isSome()) {
      return error;
    }
  }
  return None();
}
{code}

I think using functions + binding the arguments will lead to something like 
`std::initializer_list<std::function<Option<Error>()>> validators` which I 
think is less safe since we can put any function with that signature in it 
whereas the inheritance approach allows us to  express exactly which classes 
are validators via the type system.

> Add tests for validation visitors in master
> -------------------------------------------
>
>                 Key: MESOS-1064
>                 URL: https://issues.apache.org/jira/browse/MESOS-1064
>             Project: Mesos
>          Issue Type: Improvement
>          Components: master, test
>            Reporter: Dominic Hamon
>            Assignee: Dominic Hamon
>            Priority: Minor
>              Labels: c++, newbie, unit-test
>
> The various TaskInfoVisitor and OfferVisitor structures that are used for 
> validation are untested. They should be pulled out from master.cpp into their 
> own header(s) and unit tests need to be added.
> A rename to *Validator would be useful to as that's what they're used for.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to