Skip to content

[FEATURE] Add a utility class to calculate selector specificity #1049

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

Merged
merged 4 commits into from
Mar 2, 2025

Conversation

oliverklee
Copy link
Collaborator

Calculating and caching the specificity of a selector is a different concern than representing a selector, and it deserves to be in its own class.

This also helps solve the problem of selectors having keep their specificity cached and in sync with the selector itself.

(We'll have a later change that changes Selector to use the new class.)

@coveralls
Copy link

coveralls commented Mar 1, 2025

Coverage Status

coverage: 56.075% (+0.3%) from 55.799%
when pulling d0942df on feature/dependency-calculator
into f912e71 on main.

Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat solution.

However, I'm not sure about the class name, as "dependency" has no known meaning in this context that I'm aware of. I think SelectorSpecificityCalculator would be more accurate. It could be in a child namespace so it becomes Selector\SpecificityCalculator. Either way, if the class name says what it is, the method and property names can then be simply calculate() (or perhaps getForSelector()) and $cache, as that it's for specificities would be implied by the class name.

My only other (minor) concern would be that by being static, it would build up a (small) memory footprint that would remain even after all use of the library was done with. There ought to be a way of having the dependent cache instantiated on creation of the first Selector, and destroyed upon destruction of the last Selector, but I can't think of anything (other than for the Selectors to maintain a count of how many of them there are, which seems ridiculous). So there might be a requirement for a public method to clear the cache.

Calculating and caching the specificity of a selector is a
different concern than representing a selector, and it deserves
to be in its own class.

This also helps solve the problem of selectors having keep
their specificity cached and in sync with the selector itself.

(We'll have a later change that changes `Selector` to use the
new class.)
@oliverklee oliverklee force-pushed the feature/dependency-calculator branch from a63d79e to 45cc044 Compare March 2, 2025 14:14
@oliverklee
Copy link
Collaborator Author

However, I'm not sure about the class name, as "dependency" has no known meaning in this context that I'm aware of.

Me neither … I think I must have been really tired last night when I created that class. 😴

@oliverklee
Copy link
Collaborator Author

Updated and repushed.

@oliverklee oliverklee requested a review from JakeQZ March 2, 2025 14:22
@JakeQZ JakeQZ merged commit a037425 into main Mar 2, 2025
21 checks passed
@JakeQZ JakeQZ deleted the feature/dependency-calculator branch March 2, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants