2

I have two components, both are a parser/builder pair. So component A can parse A and build A, component B can parse B and build B.

A and B both contain different entities, which are extracted by the parsers.

Example, BParser works the same:

$AParserResult = AParser::parse(A);
dump(
    $AParserResult->getEntityZ(),
    $AParserResult->getEntityX(),
    $AParserResult->getEntityY(), 
    ...
);

$aBuilderSpecification = ABuilderSpecification::new()
    ->addEntityZ(Z)
    ->addEntityY(Y)  
    ->addEntityX(X);

A = ABuilder::build($aBuilderSpecification);
// now I have a "fully functionally" A again

My task is to write component C that uses AParser to parse an A, then create a BBuilderSpecification from the AParserResult and then pass that to the BBuilder and get a B from it - and the other way round, creating a B from an A.

The parsers/builders are relatively straightforward to use, however, my code in component C mostly looks like this:

// in method 'transformAToB()'

$AParserResult = AParser::parse(A);
$BBuilderSpecification = BBuilderSpecification::new();

if ($AParserResult->hasEntityZ()) {
    $BBuilderSpecification->setEntityZ($AParserResult->getEntityZ());
}

if ($AParserResult->hasEntityX()) {
    $BBuilderSpecification->setEntityX($AParserResult->getEntityX());
}

// sometimes some simple conditions maybe, and simple transformations
if ($AParserResult->hasEntityY() && someCondition()) {
    $BBuilderSpecification->setEntityY(to_lower_case($AParserResult->getEntityY()));
}

...

// Goes on for about 10 more entities
// and very similar code in the method 'transformBToA'

This is very repetitive code - I wonder is there any way to clean that up/refactoring it - without introducing additional complexity by adding an XML configuration or so … in the end it's (almost) only boilerplate code.

Language is PHP, if that matters.

  • Create an enumeration of entities and put an entity collection in your parse result base class that you can iterate over? – Martin Maat Nov 17 '16 at 5:16
1

First step: introduce the transform map

Your:

dump(
    $AParserResult->getEntityZ(),
    $AParserResult->getEntityX(),
    $AParserResult->getEntityY(), 
    ...
);

example is unfortunately not very specific, but it may make sense to create an additional getter getEntities which would return a dictionary of all entities, in this case Z, X and Y. Similarly, a new builder method addEntities($entities) will accept a dictionary containing multiple entities.

Then there is a transform step. Given your example:

if ($AParserResult->hasEntityZ()) {
    $BBuilderSpecification->setEntityZ($AParserResult->getEntityZ());
}

if ($AParserResult->hasEntityX()) {
    $BBuilderSpecification->setEntityX($AParserResult->getEntityX());
}

// sometimes some simple conditions maybe, and simple transformations
if ($AParserResult->hasEntityY() && someCondition()) {
    $BBuilderSpecification->setEntityY(to_lower_case($AParserResult->getEntityY()));
}

the entities Z and X are mapped as is, while Y is more tricky:

  • It has the to_lower_case function applied to it,

  • It is mapped only when a condition is matched.

Another way to see it is that Z and X are just specific cases where:

  • The function is the identity,

  • The condition is simply if (true);.

The resulting code looks like this:

if ($AParserResult->hasEntityZ() && conditionZ()) {
    $BBuilderSpecification->setEntityZ(transformZ($AParserResult->getEntityZ()));
}

if ($AParserResult->hasEntityX() && conditionX()) {
    $BBuilderSpecification->setEntityX(transformX($AParserResult->getEntityX()));
}

if ($AParserResult->hasEntityY() && conditionY()) {
    $BBuilderSpecification->setEntityY(transformY($AParserResult->getEntityY()));
}

You can then move the conditions and the transform functions to a map. Combined with the previously created getEntities and addEntities, you end up with the following code:

$AParserResult = AParser::parse(A);
$BBuilderSpecification = BBuilderSpecification::new();

$transformRules = ... // Load the map here.

$allEntities = $AParserResult->getEntities();
$filtered = array_filter($allEntities, $transformRules->canUse);
$transformed = array_map($transformRules->transform, $filtered);
$BBuilderSpecification->setEntities($transformed);

Second step: move the logic from map to entities themselves

The code above remains unchanged when you add new types of entities, which is good. However, the map has to be changed, and that's not good at all. The problem is that when you're adding a new type of entity, you'll have absolutely no clue that somewhere, in the future, this change will lead to a bug; tracing the bug to the change won't be easy either.

There is a better alternative.

You may create classes for every entity type. For example, entity Z and X will be instances of class α, and entity Y will be an instance of class β. Both α and β will implement a common interface which will then be used by transformAToB method.

interface IEntity
{
    public function canUse();
    public function transform();
}

class α implements IEntity
{
    ...
    public function canUse() {
        return true;
    }

    public function transform() {
        return $this->value;
    }
}

class β implements IEntity
{
    ...
    public function canUse() {
        ... // Implement whatever logic was in `someCondition()`.
    }

    public function transform() {
        return to_lower_case($this->value);
    }
}

Now the component C code looks like this:

$AParserResult = AParser::parse(A);
$BBuilderSpecification = BBuilderSpecification::new();

$allEntities = $AParserResult->getEntities();

$filtered = array_filter($allEntities, function ($entity) {
    return $entity->canUse();
});

$transformed = array_map(function () {
    return $entity->transform();
}, $filtered);

$BBuilderSpecification->setEntities($transformed);

Now when you add entity W which has a different logic from Z, X and Y, there is no map to modify: all you have to do is to include the new logic within the new class.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.