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] Clarify what parameters are passed in #21063

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 8, 2021

Overview

[Ref] Clarify what parameters are passed in

this function is called from 2 places with 4 keys in the array always set & no others

Before

It's not clear in the function that these 3 values will always be NULL

$contactId = $params['contact_id'] ?? NULL;
$componentId = $params['component_id'] ?? NULL;
$componentName = $params['componentName'] ?? NULL;

Or that the keys will always be present for

 $contributionId = $params['contribution_id'] ?? NULL;
 $contributionStatusId = $params['contribution_status_id'] ?? NULL;
 $previousContriStatusId = $params['previous_contribution_status_id'];

which means that any changes around the assumptions for those keys will require a reviewer to do a grep.

After

By tightening the setting of values from these keys we make it so a reviewer of future cleanups won't need to look outside this function to check the values coming in.

Also assigning the name to values makes the if clauses more legible - which will make it easier to point out the chunks of this code are unreachable in follow ups :-)

Technical Details

These are the 2 places that call the function and in both places exactly the same 4 keys are set

image

image

image

Comments

Obviously it's a given that external code should not be calling this function - in universe I find no examples (I do find a couple of references to a function that used to exist and used to call this function & I logged an issue against the relevant extension)

image

this function is called from 2 places with 4 keys in the array always set & no others
@civibot
Copy link

civibot bot commented Aug 8, 2021

(Standard links)

@civibot civibot bot added the master label Aug 8, 2021
@colemanw colemanw merged commit b2b2235 into civicrm:master Aug 9, 2021
@colemanw colemanw deleted the vars2 branch August 9, 2021 22:41
@colemanw
Copy link
Member

colemanw commented Aug 9, 2021

I've read the code and the refactoring makes sense.

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.

2 participants