Public-Health-Scotland / phsmethods

An R package to standardise methods used in Public Health Scotland (https://public-health-scotland.github.io/phsmethods/)
https://public-health-scotland.github.io/phsmethods/
54 stars 13 forks source link

Rename generically named functions #67

Closed Moohan closed 2 years ago

Moohan commented 3 years ago

A couple of functions have really generic names, and often are the same or very similar to variable names datasets. This leads to needlessly obscure syntax which also messes with the usefulness of RStudio auto-complete e.g.

To format a postcode variable called postcode:

data %>% 
    mutate(postcode = postcode(postcode))

To create an age_group variable:

data %>% 
    mutate(age_group = age_group(age))

The other one is fin_year.

I propose renaming the functions to something more descriptive: postcode() -> format_postcode() age_group() -> make_age_groups() fin_year() -> extract_fin_year()

To implement this, we could create the new named functions and keep the old functions as redirects with a depreciation message/warning. They can be fully deleted at a later date.

lucindalawrie commented 3 years ago

This sounds like a very sensible idea, happy for you to go ahead and create a pull request for this update. The postcode = postcode(postcode) example is one I have experienced myself a few times and although I know what it's doing, anyone else looking at my script may not understand.

Can I suggest that we change age_group() to create_age_groups() instead of make_age_groups() though?

Thanks 😃

Moohan commented 3 years ago

Happy to have a look at a PR, I can change the names to anything, I'm not attached to any of my suggestions!