Skip to content

[CLEANUP] Avoid Hungarian notation for start(Position) #977

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 2 commits into from
Feb 24, 2025

Conversation

oliverklee
Copy link
Collaborator

Part of #756

@coveralls
Copy link

coveralls commented Feb 24, 2025

Coverage Status

coverage: 53.778%. remained the same
when pulling ed47f1e on cleanup/hungarian/start
into 6f1fc2c 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.

I think $offset would be better throughout as a variable name.

* @param int $length
*/
private function substr($iStart, $length): string
private function substr($start, $length): string
Copy link
Collaborator

Choose a reason for hiding this comment

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

The equivalant PHP function has this parameter as $offset, which I think is better.

for ($iStartPosition = 0; $iStartPosition < $iStackLength; ++$iStartPosition) {
if ($iStartPosition === ($iStackLength - 1) || $sDelimiter !== $aStack[$iStartPosition + 1]) {
$aNewStack[] = $aStack[$iStartPosition];
for ($startPosition = 0; $startPosition < $iStackLength; ++$startPosition) {
Copy link
Collaborator

@JakeQZ JakeQZ Feb 24, 2025

Choose a reason for hiding this comment

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

Perhaps $offset would be a better name here too - particularly as it is changed during loop iteration to lose its original "start" meaning.

@oliverklee oliverklee force-pushed the cleanup/hungarian/start branch from 97996b5 to ed47f1e Compare February 24, 2025 11:06
@oliverklee oliverklee requested a review from JakeQZ February 24, 2025 11:06
@JakeQZ JakeQZ merged commit 7e812c8 into main Feb 24, 2025
21 checks passed
@JakeQZ JakeQZ deleted the cleanup/hungarian/start branch February 24, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants