dev/core#1116 - just rename local var activityTName

[demeritcowboy]

Overview

This is a reworking of #14972 but in baby steps.

Before

Local variable called $activityTName is actually label and is really a plural (an array, although unless something is weird in the database it will only have one element). It isn't used outside this block.

After

Changed it to $activityTypeHumanLabels.

Technical Details

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@demeritcowboy well this one doesn't say 'do not merge yet' :-)

I would vote for dropping the word humans - might be too sapien centric :-) (But actually I think label is enough - we don't have a problem with too many things being called 'labels' just with labels being misnamed as names too much)

[demeritcowboy]

@eileenmcnaughton I'm not dead-set on "human" but I would like to qualify it with something to distinguish it from Name which means almost the same thing in english as Label to "normal" people. I'm trying to look at it from the point of view of future developers who aren't knee-deep familiar, and where I'm coming from is the list of things that lead to this situation which point to such confusion (e.g. https://lab.civicrm.org/dev/core/issues/1046#note_21303 or https://lab.civicrm.org/dev/core/issues/774#note_14531).

I was also thinking that for name I'd use activityTypeMachineName - the word machine name is used elsewhere in the code in some places so there's some precedent. So "human" seems like the right complement to "machine".

Is there concern about the length of the variable? Maybe activity is a bit redundant in the context and could be just aTypeHumanLabel or actHumanLabel?

Personally I find underscores make variable names more readable but that doesn't seem to be the standard in civi, e.g. activity_type_something_label.

activityTypeDisplayLabel seems like an ok choice except unfortunately in the same block of code it refers to displayName for contacts which could be kind of confusing. Or not, since it's clearly about contacts.

[eileenmcnaughton]

@demeritcowboy personally I favour long labels over abbreviations... I think abbreviations can be a lot less obvious that they seem at the time

[demeritcowboy]

Ok. If "human" is the problem then what do you think about activityTypeDisplayLabel, since I can move the block of code that talks about displayName into its own little function to visually separate it to avoid mixing the words in the same block? Although I still do kind of lean towards human as the natural complement to machine, despite it offending the Martians.

Or just let this sit for a bit if any other ideas come up.

[eileenmcnaughton]

I'm + 1 on activityTypeDisplayLabel

[eileenmcnaughton]

test this please

[demeritcowboy]

Thanks @eileenmcnaughton. I'll rejig the next increment when I get home, which will be later in your timezone.