Simplify returns #2980
Simplify returns #2980
Conversation
|
Should I apply the CS, or there will be a PR just for that? |
| } | ||
|
|
||
| return true; | ||
| return ! ($index1->isFullfilledBy($index2) && $index2->isFullfilledBy($index1)); |
There was a problem hiding this comment.
I'd argue this approach is better than the original one. When debugging code, it may be convenient to set a break point on one of the returns and inspect the scope before switching the context. Especially, when the condition is complex. A better rework could look like:
if ( ! $index1->isFullfilledBy($index2)) {
return true;
}
if ( ! $index2->isFullfilledBy($index1)) {
return true;
}
return false;There was a problem hiding this comment.
May I do this change in another PR? Keeping one proposal per PR? I agree with you, and there are other places that we can change it 😄
There was a problem hiding this comment.
It's fine if this change is removed from here and resolved as part of another patch.
There was a problem hiding this comment.
I find this much more readable and simpler to be honest:
return ! $index1->isFullfilledBy($index2) || ! $index2->isFullfilledBy($index1);
There was a problem hiding this comment.
@lcobucci that's just De Morgan's law applied (except that @carusogabriel's is the "simplified" version ;-) )
There was a problem hiding this comment.
"De Morgan's law"
Learn something new today 😄
| } | ||
|
|
||
| return false; | ||
| return $this->spansColumns($other->getColumns()) && ($this->isPrimary() || $this->isUnique()) && $this->samePartialIndex($other); |
There was a problem hiding this comment.
Same as above, this condition could be split into a series for better readability.
I believe, all tickets marked as "Code Style" should be reflected in the coding standards if technically possible. Otherwise, it will be a never-ending story of writing new code and fixing the style. Also, having it standardized will help make sure all contributors agree with the code style, not only the ones involved in work on a given pull request. |
|
@morozov Shipping this for now. I think we should get @carusogabriel to propose these in the https://github.com/doctrine/coding-standard once he's joined the party :-) |
No description provided.