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.

Sign up for GitHub

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

Jump to bottom

define REDCap constants #217

Closed
wibeasley opened this issue Jun 2, 2018 · 3 comments
Closed

define REDCap constants #217

wibeasley opened this issue Jun 2, 2018 · 3 comments
Assignees
@wibeasley
Labels
enhancement

Comments

@wibeasley
Copy link
Member

wibeasley commented Jun 2, 2018 •
edited
Loading

centrally define constants. Right now, I'm frequently re-defining these at the top of a lot of R files to avoid magic constants sprinkled throughout the code. There could be more than this, but currently I'd use

  • form_incomplete = 1 0
  • form_unverified = 2 1
  • form_complete = 3 2

These values might be contained in a single list called constant, so the caller's code might look like

ds <- ds %>%
  dplyr::mutate(
    bookkeeping_complete   = REDCapR::constant$form_unverified
  )

Alternatively, there could be a function called constant(), that accepts the name. The function would check to see if the constant is even defined, instead of returning NULL. The caller's code might look like

ds <- ds %>%
  dplyr::mutate(
    bookkeeping_complete   = REDCapR::constant("form_unverified")
  )

Any opinions? If anyone has experience with this, or might have a better way, please tell me.
@wibeasley wibeasley self-assigned this Jun 2, 2018
@nutterb
Copy link
Contributor

nutterb commented Jun 2, 2018

I would define each constant separately. As long as they aren't exported, it won't create any issues for the user.

For your own sanity, I'd recommend naming them with an underscore in front, just to give a visual cue that it is an internal constant. So _form_incomplete = 1, for example.

@wibeasley
Copy link
Member Author

@nutterb, I was thinking I would export them, so all my external code wouldn't have to define them. Do you think that's a bad idea?

wibeasley added a commit that referenced this issue Jun 3, 2018
@wibeasley
constant -scalars
70cadf5 
ref #217
@nutterb
Copy link
Contributor

nutterb commented Jun 4, 2018

Not a bad idea at all. It just escaped me that you intended this for direct user access (I'm sleep deprived).

I like how you have this set up currently, with constant_list being unexported, but accessible through a function.

I might add checkmate::assert_logical(simplify, len = 1) to your checks.

wibeasley added a commit that referenced this issue Jun 4, 2018
@wibeasley
validated simplify
ebbc8d8 
Thanks, @nutterb. Ref #217
wibeasley added a commit that referenced this issue Jun 4, 2018
@wibeasley
tolerate different versions of checkmate
2f65173 
ref #217
wibeasley added a commit that referenced this issue Sep 17, 2018
@wibeasley
Add constants related to user rights
602cf4e 
ref #163; ref #217
wibeasley added a commit that referenced this issue Sep 17, 2018
@wibeasley
better documentation of constants
91f7184 
ref #217
@wibeasley wibeasley mentioned this issue Dec 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wibeasley wibeasley

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@wibeasley @nutterb