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

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

Comments

@wibeasley
Copy link
Member

wibeasley commented Jun 2, 2018

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
@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 added a commit that referenced this issue Jun 4, 2018
wibeasley added a commit that referenced this issue Sep 17, 2018
wibeasley added a commit that referenced this issue Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants