assert icon indicating copy to clipboard operation
assert copied to clipboard

Add implies as logical assertion

Open roelvanduijnhoven opened this issue 3 years ago • 6 comments

I extracted this logical assertion from our code-base. It helps us in cases where we want to assert transitions between the states something can be in. But it can be used in more situations.

This is en existing logical operator, see: https://en.wikipedia.org/wiki/Material_conditional. Not sure how well known it is, but it makes sense once you start using it.

So: let's say you want to assert the input of method updateState($old, $new):

  • You want it to only allow you to go from PENDING to ACTIVE, you can assert using:

    Assert::implies($old === PENDING, $new === ACTIVE);
    

    Which in speech would translate to: If the old status is PENDING, the new status must be ACTIVE.

    And you could also write this as follows, but note how that does not communicate the actual intent.

    Assert::true($old !== PENDING || $new === ACTIVE);
    
  • Or you want to assert that you can only reach TERMINATED either from ACTIVE or PENDING, that would be;

    Assert::implies($new === TERMINATED, in_array($old [ACTIVE, PENDING]));
    

    Which in speech would translate to: If the new status is TERMINATED the old status must be either ACTIVE or PENDING.

    And you could also write this as follows:

    Assert::true($new !== TERMINATED || !in_array($old [ACTIVE, PENDING]));
    

Would love to hear if anybody thinks this is something you consider adding to this repo.

roelvanduijnhoven avatar Dec 01 '20 11:12 roelvanduijnhoven

Tests fail. I'll wait to hear feedback on this PR before I fix those.

roelvanduijnhoven avatar Dec 01 '20 11:12 roelvanduijnhoven

Could you please rebase this PR to make sure the tests are green?

Nyholm avatar Jan 18 '21 12:01 Nyholm

Allright @Nyholm I rebased and squashed my commits. Tests are now green. Only the Code Coverage fails, but it looks like this is not related to what I have done.

The requested page was not found, or you have no access to view it.

roelvanduijnhoven avatar Jan 25 '21 16:01 roelvanduijnhoven

to @ all

what do you think about changing the signature to

    /**
     * @psalm-pure
     *
     * @param bool $p
     * @param bool $q
     * @param string $message
     *
     * @throws InvalidArgumentException
     */
    public static function implies($p, $q, $message = '')

I personally hate type coercion and think that negation of mixed is always GIGO.

Thoughts?

zerkms avatar Jan 26 '21 03:01 zerkms

@zerkms I just copied what I already found out in the existing code base.

However I think this is done to support stuff like:

Assert::implies(count($people), count($seats))

I.e.: if there are people, there should be seats.

Anyhow: let me know if I do need to change to booleans.

roelvanduijnhoven avatar Jan 27 '21 07:01 roelvanduijnhoven

Assert::implies(count($people) > 0, count($seats) > 0)

^ this code expresses the domain requirement explicitly: if there are any people - then there should be some seats. To me if (0) (in languages with boolean type available) makes very little sense.

in the existing code base

In other places some extra assertions should be applied then as well.

let me know if I do need to change to booleans.

I'm just another contributor here and don't make decisions on my own :-)

zerkms avatar Jan 27 '21 08:01 zerkms