Add basic comparison helper functions #5
Conversation
| use Composer\Semver\Constraint\VersionConstraint; | ||
| use InvalidArgumentException; | ||
|
|
||
| class Compare |
There was a problem hiding this comment.
Maybe this class should be final?
There was a problem hiding this comment.
Can you expand on your suggestion? What would be the drawback of allowing extending?
There was a problem hiding this comment.
Comaprator would be better than Compare IMO. Class names are better as noun than as verbs
There was a problem hiding this comment.
You mean Comparator? Or how about Comparison?
There was a problem hiding this comment.
yeah, sorry for the typos. I typed too fast
There was a problem hiding this comment.
Ok, renamed to Comparator, but not final.
|
Do you want to look into implementing the remaining methods as well? And if so, in this PR or another? |
| * Evaluate $version1 $comparison $version2. | ||
| * | ||
| * @param string $version1 | ||
| * @param string $comparison Operator like ">" or "<=", etc. |
There was a problem hiding this comment.
I would rename the argument to $operator
I will submit a different PR for the other methods. |
|
What about calling this class just |
I am pretty terrible at naming things, so I typically defer to other people :P. So I can change it to whatever you and stof agree on...but we can also go with the current one and change the class name to a better one in the future before we make an initial release. |
|
I personally find Comparator a bid awkward. But on the other hand it fits the context I guess. So I don't know. |
|
I'd be fine with Composer\Semver\Semver as high level API. However the function names are all over the place. Only two have a To suffix, that should be unified one way or the other. And the greaterThanEqualTo IMO should have an |
|
I'd prefer Comparator to be honest, that's quite precisely what it is, and we have Semver in the namespace already, so that part is clear too. |
|
But would you also add functions that handle increasing/decreasing versions etc into |
|
Maybe Comparator::compare and the rest stays on a Semver class as shortcuts? |
|
Could you define "rest" and "shortcuts"? I'm a bit confused as to what you are referring to now. |
👍 |
|
rest: all methods except compare. But yeah we can as well just keep it all in Comparator IMO. We'll need another class(es) for the other methods though, if we want to implement them at all. |
Part of composer#2. Implements helper functions for greater than, greater than or equal to, less than, less than or equal to, equal to, and not equal to. They all call Comparator::compare() which takes an arbitrary comparison operator.
Add basic comparison helper functions
Added "Or" to those two functions. The lack of the "To" suffix on lessThan and greaterThan is intentional, those typically aren't used with "to" in English (e.g. https://en.wikipedia.org/wiki/Inequality_%28mathematics%29). |
|
Thanks :-) |
Part of #2.
Implements helper functions for greater than, greater than or equal to,
less than, less than or equal to, equal to, and not equal to.
They all call Compare:compare() which takes an arbitrary comparison
operator.