Skip to content

Readability #81

@drymek

Description

@drymek

It is quite hard to understand the current structure of the Specification. When you look at:

$spec = Spec::andX(
    Spec::eq('ended', 0),
    Spec::orX(
        Spec::lt('endDate', new \DateTime()),
        Spec::andX(
            Spec::isNull('endDate'),
            Spec::lt('startDate', new \DateTime('-4weeks'))
        )
    )
);

return $this->em->getRepository('HappyrRecruitmentBundle:Advert')->match($spec);

it's quite similar to Doctrine version (in the context of readability). I think it could be improved by such creation Specifications:

$spec = (new Property('ended'))->equals(0)->and(
    (new Property('endDate'))->lessThan(new \DateTime())->or(
        (new Property('endDate'))->isNull()->and(
            (new Property('startDate'))->lowerThan(new \DateTime('-4weeks'))
        )
    )
);

advantage is that you can read it:
Property ended is equals to "0" and property end date i less than DateTime (or $now = new \DateTime) (...). But it also has disadvantages:

  • Property class is a little bit virtual,
  • there is no new property at all,
  • it doesn't have to be a property,
  • implementation may seems tricky
  • and probably something else I did not notice

Recipe

  • Introduce new Property class:
class Property
{
    use Conditions;

    public function __construct($name)
    {
        $this->name = $name;
    }
}
  • Adding this trait to each filter (Equals, LessThan, isNull etc) and to Property class:
trait Conditions
{
    protected $propertyName;

    public function equals($value)
    {
        return new Equals($this->propertyName, $value);
    }

    public function lessThan($value)
    {
        return new LessThan($this->propertyName, $value);
    }

    public function isNull()
    {
        return new IsNull($this->propertyName);
    }
}
  • Adding this trait to LogicX class:
trait LogicSpecification
{
    public function and($value)
    {
        return new AndX($this, $value);
    }

    public function or($value)
    {
        return new OrX($this, $value);
    }
}

after correcting any typos it should work. Voilà.

Conclusion

This may not be the best solution, or it may be even worse than the existing one. I would like to ask you to comment on readability and/or alternative ideas.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions