Chained creational OOP design pattern problem

Question

I am creating a class that is responsible for validating a configuration. This class calls other classes that validate said config by creating new instances in the form of a chain. At first glance, the code structure looks horrible, but It works. Anyway, I think it's not the best way to handle this logic.

I leave here a simplified version of the code in TypeScript, but I also leave it in Python and Java for reference only:

class Validator {
    private _notValidatedConfig: NotValidatedConfig

    constructor(notValidatedConfig: NotValidatedConfig) {
        this._notValidatedConfig = notValidatedConfig
    }

    validateConfig(): ValidatedConfig {
        return (
            new Phase4Validation(
                new Phase3Validation(
                    new Phase2Validation(
                        new Phase1Validation(
                            this._notValidatedConfig
                        ).validate()
                    ).validate()
                ).validate()
            ).validate()
        )
    }

    // Alternative
    validateConfig2(): ValidatedConfig {
        const validatedPhase1Config: ValidatedPhase1Config = new Phase1Validation(this._notValidatedConfig).validate()
        const validatedPhase2Config: ValidatedPhase2Config = new Phase2Validation(validatedPhase1Config).validate()
        const validatedPhase3Config: ValidatedPhase3Config = new Phase3Validation(validatedPhase2Config).validate()
        const validatedPhase4Config: ValidatedPhase4Config = new Phase4Validation(validatedPhase3Config).validate()
        return validatedPhase4Config;
    }
}

Python
Java Disclaimer: I don't have any experience with Java, so maybe there are some syntax errors.

The "alternative" is the same code, but not directly chained, instead, for every validation, it's creating a new variable. I think the "alternative" is more readable but performs worse.

What do you think about this code? what did you change? How would you face this problem or with what design pattern or framework? (programming language doesn't matter for these question)

@dangerousmanleesanghyeon that's a good one. I left an answer based on the "Chain of Responsibility" design pattern, but I think I misunderstood some concepts and ended up implementing a decorator pattern. — Lucas Vazquez, Jun 13 '22 at 00:41
Questions about working code are more appropriate for https://codereview.stackexchange.com/. — jaco0646, Jun 13 '22 at 03:15

StepUp · Answer 1 · 2022-06-17T05:46:54.177

I would create a base class Validation and just create derived classes from it if it is necessary to add new validation:

public abstract class Validation
{
    public Validation(string config)
    {

    }

    public abstract string Validate();
}

and its concrete implementations:

public class Phase1Validation : Validation
{
    public Phase1Validation(string config) : base(config)
    {}

    public override string Validate()
    {
        if (true)
            return null;

        return "There are some errors Phase1Validation";
    }
}

public class Phase2Validation : Validation
{
    public Phase2Validation(string config) : base(config)
    {

    }

    public override string Validate()
    {
        if (true)
            return null;

        return "There are some errors in Phase2Validation";
    }
}

and then just create a list of validators and iterate through them to find errors:

public string Validate()
{
    List<Validation> validations = new List<Validation>()
    {
        new Phase1Validation("config 1"),
        new Phase2Validation("config 2")
    };

    foreach (Validation validation in validations)
    {
        string error = validation.Validate();
        if (!string.IsNullOrEmpty(error))
            return error;
    }

    return null; // it means that there are no errors
}

UPDATE:

I've little bit edited my classes to fit your new question requirements:

validations should be ordered. Added Order property
get config from previous validation and send it to the next validation

It can be seen that this approach allows to avoid to write nested classes like this:

new Phase4Validation(
   new Phase3Validation(
      new Phase2Validation(...).validate()
   ).validate()
).validate()

So you can add new classes without editing validation classes and it helps to keep Open CLosed Principle of SOLID principles.

So the code looks like this:

Abstractions:

public abstract class Validation
{
    // Order to handle your validations
    public int Order { get; set; }

    // Your config file
    public string Config { get; set; }

    public Validation(int order)
    {
        Order = order;
    }



    // "virtual" means that method can be overriden
    public virtual string Validate(string config) 
    {
        Config = config;

        if (true)
            return null;

        return "There are some errors Phase1Validation";
    }
}

And its concrete implementations:

public class Phase1Validation : Validation
{
    public Phase1Validation(int order) : base(order)
    {
    }
}

public class Phase2Validation : Validation
{
    public Phase2Validation(int order) : base(order)
    {
    }
}

And method to validate:

string Validate()
{
    List<Validation> validations = new List<Validation>()
    {
        new Phase1Validation(1),
        new Phase2Validation(2)
    };

    validations = validations.OrderBy(v => v.Order).ToList();
    string config = "";
    foreach (Validation validation in validations)
    {
        string error = validation.Validate(config);
        config = validation.Config;
        if (!string.IsNullOrEmpty(error))
            return error;
    }

    return null; // it means that there are no errors
}

Thanks for your answer!, although it's not much different from mine. The real problem I am figuring out is that each phase is strictly dependent (by type) on the previous one. Phase 1 receives a config and validates it. Once validated, this config is consumed by the phase 2 validator. I need to strictly specify this in such a way that if I consume the config directly from phase 2 (without consuming it first from phase 1) an error emerges. — Lucas Vazquez, Jun 15 '22 at 21:46
Maybe, I need another design. I can simply create a single class with private methods like `Phase1Validation`, `Phase2Validation`, ...`PhaseNValidation`, and then, make a method called `Validate` and call each phase in a chained way. E.g. https://pastebin.com/43KDeYAp . Anyway, I think this approach is worse than the one I originally posted in the question. — Lucas Vazquez, Jun 15 '22 at 22:00
@LucasVazquez please, see my updated answer. There is a difference that there is no nesting of your classes which you wrote *At first glance, the code structure looks horrible, but It works.*. I've reedited my code, so you can add an ordering and you can send a config for your classes. — StepUp, Jun 17 '22 at 05:48
Thanks for your update, I really appreciate your time. This other answer that you have given me is curious and I like it! — Lucas Vazquez, Jun 19 '22 at 00:57

Lucas Vazquez · Answer 2 · 2022-06-14T22:13:55.853

I leave here my own answer, but I'm not going to select it as correct because I think there exist better answers (besides the fact that I am not very convinced of this implementation).

A kind of Decorator design pattern allowed me to do chain validation with greater use of the dependency injection approach. I leave here the code but only for Python (I have reduced the number of phases from 4 to 2 to simplify the example).

from __future__ import annotations

import abc
from typing import cast
from typing import Any
from typing import TypedDict


NotValidatedConfig = dict
ValidatedConfig = TypedDict("ValidatedConfig", {"foo": Any, "bar": Any})


class InvalidConfig(Exception):
    ...


# This class is abstract.
class ValidationHandler(abc.ABC):
    _handler: ValidationHandler | None

    def __init__(self, handler: ValidationHandler = None):
        self._handler = handler

    # This method is abstract.
    @abc.abstractmethod
    def _validate(self, not_validated_config: NotValidatedConfig):
        ...

    def _chain_validation(self, not_validated_config: NotValidatedConfig):
        if self._handler:
            self._handler._chain_validation(not_validated_config)
        self._validate(not_validated_config)

    def get_validated_config(self, not_validated_config: NotValidatedConfig) -> ValidatedConfig:
        self._chain_validation(not_validated_config)

        # Here we convert (in a forced way) the type `NotValidatedConfig` to
        # `ValidatedConfig`.
        # We do this because we already run all the validations chain.
        # Force a type is not a good way to deal with a problem, and this is
        # the main downside of this implementation (but it works anyway).
        return cast(ValidatedConfig, not_validated_config)


class Phase1Validation(ValidationHandler):

    def _validate(self, not_validated_config: NotValidatedConfig):
        if "foo" not in not_validated_config:
            raise InvalidConfig('Config miss "foo" attr')


class Phase2Validation(ValidationHandler):

    def _validate(self, not_validated_config: NotValidatedConfig):
        if not isinstance(not_validated_config["foo"], str):
            raise InvalidConfig('"foo" must be an string')


class Validator:
    _validation_handler: ValidationHandler

    def __init__(self, validation_handler: ValidationHandler):
        self._validation_handler = validation_handler

    def validate_config(self, not_validated_config: NotValidatedConfig) -> ValidatedConfig:
        return self._validation_handler.get_validated_config(not_validated_config)


if __name__ == "__main__":
    # "Pure Dependency Injection"
    validator = Validator((Phase2Validation(Phase1Validation())))
    validator.validate_config({"foo": 1, "bar": 1})

What is the problem with this approach?: the lightweight way in which the types are concatenated. In the original example, the Phase1Validation generates a ValidatedPhase1Config, which is safely used by the Phase2Validation. With this implementation, each decorator receives the same data type to validate, and this creates safety issues (in terms of typing). The Phase1Validation gets NotValidatedConfig, but the Phase2Validation can't use that type to do the validation, they need the Phase1Validation.

Chained creational OOP design pattern problem

2 Answers2