search

composer / semver

Semantic versioning utilities with the addition of version constraints parsing and checking.
MIT License
3.15k stars 76 forks source link

Replace useless method call #93

Closed jderusse closed 4 years ago

jderusse commented 4 years ago

Removes call to internal method by using properties directly. Unless we called methods because expect this class to be extended.

Note: Not a huge difference (bounds are called "only" ~1000 times)

codecov-io commented 4 years ago

Codecov Report

Merging #93 into master will decrease coverage by 0.69%. The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #93      +/-   ##
============================================
- Coverage     94.59%   93.90%   -0.70%     
- Complexity      294      295       +1     
============================================
  Files             8        8              
  Lines           574      574              
============================================
- Hits            543      539       -4     
- Misses           31       35       +4
Impacted Files Coverage Δ Complexity Δ
src/Constraint/Bound.php 51.72% <42.85%> (-13.80%) 18.00 <0.00> (+1.00) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d5af5ff...9da8724. Read the comment docs.

Seldaek commented 4 years ago

I am not so sure about this.. Generally speaking it feels weird if we have getters to bypass them, and it's a possible cause for bugs later if adding any custom behavior to the getter.

I completely see it's worth it in some instances for performance reasons, but bounds are not used enough to justify this cost IMO.

GrahamCampbell commented 4 years ago

Because this class is not final and that method is not private, this would actually be a breaking change.