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

Stop passing / using object when all we need is the id #18331

Merged
merged 1 commit into from
Sep 4, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor code cleanup to be more specific about what parameters we are interested in

Before

if ($contributionParams['contribution_status_id'] === $completedContributionStatusID) {
        self::updateMembershipBasedOnCompletionOfContribution(
          $contribution,
          $changeDate
        );

but we only use the id from contribution

After

if ($contributionParams['contribution_status_id'] === $completedContributionStatusID) {
self::updateMembershipBasedOnCompletionOfContribution(
$contributionID,
$changeDate
);

Technical Details

Rather than set id on the contribution object just to be able to access it via contribution->id
let's name the param we keep using & use that. Note I'm still getting contribution->id from contribution
here but I think this makes it clear that the object is mostly only used in addActivity now.

The slightly larger change is in updateMembershipBasedOnCompletionOfContribution where there is
an instantiation of 'self()' since we no longer have the object

Comments

@civibot
Copy link

civibot bot commented Sep 3, 2020

(Standard links)

Rather than set id on the contribution object just to be able to access it via contribution->id
let's name the param we keep using & use that. Note I'm still getting contribution->id from contribution
here but I think this makes it clear that the object is mostly only used in addActivity now.

The sligtly larger change is in updateMembershipBasedOnCompletionOfContribution where there is
an instantiation of 'self()' since we no longer have the object
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.

2 participants