API could be extended to be more fluent

[happyr]

API could be extended to be more fluent

[Nyholm]

Hi @AndyBursh
Thank you for this issue. It is a valid problem you are raising. I have not considered that one before.

I like that you provided clear code examples.

I have one question in mind. What if we added a LogicX::Append(Specification) method? I think that would solve your problem as well, wouldn't it?

public function search($parameters)
{
    $spec = new Spec::andX();
    // ... If foo is set
    $spec->append(Spec::eq("foo", $parameters["foo"]));
    // ... If bar ir set
    $spec->append(Spec::eq("bar", $parameters["bar"]));

    $finalSpec = Spec::orX(
        $spec,
        Spec::andX(Spec::lt("fizz", 4), Spec::eq("bing", false)),
        Spec::gt("buzz", 99)
    );

    $results = $repo->match($finalSpec);
}

[AP-Hunt]

@Nyholm I think adding an append spec solves the problem on at a technical level, but I'm not so sure about solving it at a usability level.

My mental model when working with specifications is of combining logical statements together, so I don't feel that appending one specification to another is a natural fit here. I think a better form of your solution might be to have correpsonding methods on the logic specifications

public function search($parameters)
{
    $spec = Spec::andX();
    // ... If foo is set
    $spec->andX(Spec::eq("foo", $parameters["foo"]);
    // ... If bar is set
    $spec->andX(Spec::eq("bar", $parameters["bar"]);

    $finalSpec = Spec::orX(
        $spec,
        Spec::andX(Spec::lt("fizz", 4), Spec:: eq("bing", 
    );
    $finalSpec->orX(Spec::gt("buzz", 99));
}

However, this then precludes me from writing something as simple as below. I'm not sure that's a bad thing, necessarily, but I think it's something worth considering.

$spec = Spec::eq("foo", $parameters["foo"]);
$spec->andX(Spec::eq("bar", $parameters["bar"]));

I could prepare implementations of both versions (fluent, and correpsonding methods) if that would help.

[Nyholm]

Hm. I see you point and I do think the code you supplied looks better.

Me and @cakper did some work and had a long discussion about #39 yesterday. He is going to provide a PR with some refactoring. Maybe an other solution will be more obvious by that PR.

I suggest we reconsider this issue after we see the result of that PR. Because I do think you raise an important issue here.

[Nyholm]

I really do want to tackle this problem now.
Consider the following scenario:

$spec = // Some spec object
$spec->andX(A);
$spec->andX(B);
$spec->orX(C);

How should the spec tree be built?

1) (A && B) || C
2) A && (B || C)

What if we use multiple arguments?

$spec = // Some spec object
$spec->andX(A, B);
$spec->orX(C);

How should the spec tree be built?

1) (A && B) || C
2) A && (B || C)

---

Suggestion

We add AndX::andX that will take one Specification as argument. Likewise we add OrX::orX.

$spec = Spec::andX(A, B)
$spec->andX(C);

// $spec will be the same as Spec::andX(A, B, C);

$spec = Spec::andX(A, B)
$spec->andX(Spec::orX(C, D));

// $spec will be the same as Spec::andX(A, B, Spec::orX(C, D));

You could now do...

public function search($parameters)
{
    $spec = Spec::andX();
    // ... If foo is set
    $spec->andX(Spec::eq("foo", $parameters["foo"]);
    // ... If bar is set
    $spec->andX(Spec::eq("bar", $parameters["bar"]);

    $finalSpec = Spec::orX(
        $spec,
        Spec::andX(Spec::lt("fizz", 4), Spec:: eq("bing", 
    );
    $finalSpec->orX(Spec::gt("buzz", 99));
}

But you can't do

$spec = Spec::andX(A);
$spec->andX(B);
$spec->orX(C); //orX is not defined on class AndX.

What do you think. Will it be more fluent? Will it be clear how the logic tree is build?

[AP-Hunt]

I think, in your first example, the first tree construct is right. The second tree would be constructed with something like:

$spec = // Some spec object
$spec->andX(A);
$spec->andX(Spec::orX(B, C));

I like the solution you've proposed with AndX::andX and OrX::orX. However, I don't think what you say about your last example is worth much worry. The tree you're attempting to construct there is still feasible to construct, it's just a bit more circuitous.

$spec = Spec::orX(
    C,
    Spec::andX(A, B) // or some previously constructed spec
);

To clarify, your proposal allows usage to be less verbose than your examples too? For example

$spec = (Spec::AndX(A))->andX(B)->andX(C);

I forget if calling members on the result of something in parentheses is allowed in PHP, but you get the gist.

[Nyholm]

To clarify, your proposal allows usage to be less verbose than your examples too? For example

That is the plan. I'll submit a PR and we can evaluate.

[Nyholm]

The PR is merged. I consider this issue to be closed.

[Nyholm]

Thank you @AndyBursh for you comments! I really think it is an improvement.

[AP-Hunt]

You're welcome, @Nyholm. Thanks for taking it in to account.