-
Notifications
You must be signed in to change notification settings - Fork 144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CLEANUP] Avoid Hungarian notation in CSSBlockList
(part 2)
#851
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think renaming $aSpecificitySearch
to $specificitySearchResults
is not accurate.
src/CSSList/CSSBlockList.php
Outdated
$sComparator = $aSpecificitySearch[0]; | ||
$iTargetSpecificity = $aSpecificitySearch[1]; | ||
$comparator = '==='; | ||
$specificitySearchResults = \explode(' ', $specificitySearch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be named with something like 'parts', 'elements' or 'components'. The input string contains either a number (e.g. "1"), or an operator and a number (e.g. ">= 1"). The result goes into $result
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe $expressionParts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think that works. It's only used locally in that small section of code.
5a2b89f
to
4bf8ee8
Compare
4bf8ee8
to
afdefc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spotted another naming issue I missed when I reviewed previously.
src/CSSList/CSSBlockList.php
Outdated
switch ($sComparator) { | ||
$targetSpecificity = (int) $targetSpecificity; | ||
$selectorSpecificity = $selector->getSpecificity(); | ||
$hasMatches = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I missed this in the previous review.
I don't think "has" is correct here. The equivalent linguistic phrase is "it matches" rather than "it has matches" - i.e. "matches" is a verb rather than a noun.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better late than never. ;-) I've come up with a different name and repushed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe comparatorMatched
?
Part of #756