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

[Ref] Remove never reachable lines. #17556

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

[Ref] Remove never reachable lines.

Before

when $pending is true the first if causes a return. Handling for pending is in the code after that return but is unreachable

After

Unreachable code gone

Technical Details

There is an early return on so updateStatusId can never be true

Comments

There is an early return on  so updateStatusId can never be true
@civibot
Copy link

civibot bot commented Jun 8, 2020

(Standard links)

@civibot civibot bot added the master label Jun 8, 2020
@@ -855,23 +854,12 @@ public function processMembership($contactID, $membershipTypeID, $is_test, $chan
}
$memParams['membership_activity_status'] = ($pending || $isPayLater) ? 'Scheduled' : 'Completed';
}
//CRM-4555
if ($pending) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton are you saying that this couldn't ever be true?

$pending = ($this->_params['contribution_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending'));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 yes see how

if ($pending || in_array($currentMembership['status_id'], [
results in it returning before that line?

(I think the same may be the case in the function on the BAO class that this form used to share)

@mattwire
Copy link
Contributor

mattwire commented Jun 9, 2020

Ok, took me a little while to see what you mean but I agree. We are removing code that is never reachable because if pending was (not false) it would have already returned via an earlier if statement. This is the same on the original BAO function and it would be good if you could put up a similar PR for that while it is fresh :-) So we don't have to go through the same thought process with that.

@seamuslee001 seamuslee001 merged commit 94e9af5 into civicrm:master Jun 9, 2020
@seamuslee001 seamuslee001 deleted the renewline branch June 9, 2020 12:03
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