Skip to content
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

Make $end parameter nullable in findBetween Method #20062

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

salehhashemi1992
Copy link
Contributor

Implemented logic to default the $end parameter to the $start value when $end is null.

Copy link

what-the-diff bot commented Nov 2, 2023

PR Summary

  • Modification of 'findBetween' method in 'BaseStringHelper.php' file
    • The procedure for identifying characters between specific points in a string has been enhanced. This method now optionally accepts a parameter indicating where to stop ('end').
    • If this 'end' parameter isn't specified, the method will default to using the starting point ('start') as the endpoint.
    • The method is also more robust now: even if the 'end' parameter is unknowingly supplied as 'null', it will be reset to match the starting point.

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (01f3983) 48.02% compared to head (88101c4) 48.02%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #20062      +/-   ##
==========================================
- Coverage   48.02%   48.02%   -0.01%     
==========================================
  Files         445      445              
  Lines       43890    43892       +2     
==========================================
- Hits        21078    21077       -1     
- Misses      22812    22815       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@salehhashemi1992 salehhashemi1992 marked this pull request as ready for review November 2, 2023 17:40
@rob006
Copy link
Contributor

rob006 commented Nov 2, 2023

I'm not sure about this feature overall, but I definitely don't like current signature. Making $end nullable only make sense if passing $end is optional. Explicitly passing null as third argument only makes call more cryptic.

@salehhashemi1992
Copy link
Contributor Author

I'm not sure about this feature overall, but I definitely don't like current signature. Making $end nullable only make sense if passing $end is optional. Explicitly passing null as third argument only makes call more cryptic.

I apologize; that was my mistake.
This feature is suggested here: yiisoft/strings#118 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants