php: how to reduce the cyclomatic complexity of a method with loads of setters

user_2055097 0
Viewed 597

I have a method, which is using values of one class to set the properties of another class:

public static function fromDto(
    ProfessionArea $professionArea,
    ProfessionAreaDto $dto,
    SignatureType $signatureType
): ProfessionArea {
    if (null !== $dto->id) {
        $professionArea->setId($dto->id);
    }
    if (null !== $dto->active) {
        $professionArea->setActive($dto->active);
    }
    if (null !== $dto->name) {
        $professionArea->setName($dto->name);
    }
    if (null !== $dto->signature) {
        $professionArea->setSignature($dto->signature);
    }
    if (null !== $dto->signatureTypeId) {
        $professionArea->setSignatureType($signatureType);
    }
    if (null !== $dto->transition) {
        $professionArea->setTransition($dto->transition);
    }
    if (null !== $dto->infotextCheck) {
        $professionArea->setInfotextCheck($dto->infotextCheck);
    }
    if (null !== $dto->textblockCompleted) {
        $professionArea->setTextblockCompleted($dto->textblockCompleted);
    }
    if (null !== $dto->deletable) {
        $professionArea->setDeletable($dto->deletable);
    }

    return $professionArea;
}

phpmd complains, that the complexity of this method it too high since we can have too many variations of true or false if statement results. NPath currently has the result of 512 and should not be bigger than 200.

But how to reduce the complexity? Moving it to another method doesn't help because I moved the code already into a helper class and I have to check each value if it is set. The problem occurs in similar fashion in several of my classes.

Would looping through an array of values, which create dynamic method and property names make it better or would it just reduce readability of the code?

php-7.4 php
1 Answers

Since it seems very important that the properties of your ProfessionArea objects are not null, I would move the check against null in your setters method:

class ProfessionArea {
    public function active($active) {
        if (null !== $active) {
            $this->active= $active;
        }
    }

   //other methods here
}

Here you are showing a fromDto method to build your instances, but supposedly you could have other similar methods like fromJson, fromString, whatever, and cyclomatic complexity aside, the approach you are presenting would mean a ton of repetition of all those checks against null values.

There may be some business conditions to consider when moving the checkd against null in the setter methods (maybe in some conditions it would be fine for a property to be null), but that's up to your requirements.

That said, I can't say if phpmd would complain about something else with a similar approach.

user_8391580
Related