What would be the correct design approach?

Question

Hi,

I have following existing scenario.

I have a Validator class containing validate( Command* ) function which validates the Command passed to it.

class Validator
{
public:
   validate(Command* cmd)
   { 
        // common validation logic
   }

}

I have three classes say WindowsExecute, SolarisExecute and AIXExecute. Member function execute() in SolarisExecute and AIXExecute directly create object of Validator and use the validate( Comman* ) function for validating the Command before executing.

class SolarisExecute
{
public:
   execute(Command *cmd)
   {
        Validator v;
        bool valid = v.validate(cmd);

        // some processing depending on 'valid'
   }
}

class AIXExecute
{
public:
   execute(Command *cmd)
   {
        Validator v;
        bool valid = v.validate(cmd);

        // some processing depending on 'valid'
   }
}

WindowsExecute is completely different and does not have any Command. Instead it need to validate some string data. To do this there is a separate class called WindowsValidator inherited from Validator. WindowsExecute::execute() uses WindowsValidator instead of Validator.

class WindowsValidator : Validator
{
public:
   validate(const string &xmlData)
   {
       // specific validation logic
   }
}

class WindowsExecute
{
public:
   execute(const string &data)
   {
        WindowsValidate v;
        bool valid = v.validate(data);

        // some processing depending on 'valid'
   }
}

This is existing code.

Now I need to do some specific validations of Solaris and hence can't use Validator::validate( Command* ). Following the current design, I would need to create new class called SolarisValidator and have my own implementation of validate( Command* ).

I am not comfortable with this approach. Some issues/comments I think:

1. Validator class would be used only by AIXExecute. Then why have a base class if there is nothing common logic remaining? Simply have three classes SolarisValidator, AIXValidator, WindowsValidator.

2. Validator::validate( Command* ) unnecessarily gets inherited into WindowsValidate class. Note the signature of WindowsValidate::validate(string) and Validator::validate( Command* ) are different.

3. I should make Validator::validate( Command* ) virtual if I introduce SolarisValidator::validate( Command* ). It means I am introducing overhead of virtual pointers even though I am not using any dynamic polymorphism. So why not go with #1 above and create three separate classes?

What would be the best solution for this scenario which would also be extensible in future? I am using C++ for implementation.

Thanks in advance.

-GP

Answer 1

It looks like you have the concept of a Command, that is valid or not. Depending on your platform, the command is represented in a different form.

So I wondered: why not create an ICommand interface with a function "isValid", and have your platform-wrapping code create the proper ICommand object for that platform. This would free your "execute" call from creating a validator, and hence making it platform-independent.

Note: this platform-wrapper subsystem is imho best designed using the Factory pattern.

Answer 2

It sounds very much like you don't actually have any common functionality or even a common interface between your various ...Validator classes. Personally I would remove the common base class and only resurrect it if a genuine common interface emerges. Trying to force a common base class with no clear purpose or benefit will only lead to messy code and/or a maintenance cost without benefit.

At this point you might want to remove the ...Validator classes altogether and move the functionality into a separate function of the ...Execute classes. It sounds as though the validation is tightly bound to the implementation of the execute methods and that this is why you are having difficulty abstracting an appropriate validation interface.

Trying to uncouple things which are inherently tightly coupled is frequently an exercise in futility. On the other hand, things which aren't inherently tightly coupled shouldn't be allowed to become tightly coupled in implementation purely through bad design.