On request of @catalamarti, here is a summary of the PaRe-report. I will (or have) send the entire report to Martí.
Function diagram
mockCdmFromTables seems to kicks off a lot of functions, but is covered for 77%. Code coverage is obviously somewhat arbitrary, but considering the amount of stuff it does, I think bumping up the coverage would be nice.
Code quality
min
median
max
Number of arguments
1
3
10
Lines of code
3
12.5
114
Cyclomatic complexity
1
2
9
The summary of the code quality seems great, no large functions, relatively simple functions. Some functions have quite some arguments (>7) but nothing too concerning. The largest function seems to be mockCohort (args: 6, lines: 114, cycloComp: 8). But about a third of that is asserting the input arguments.
Because of the smaller functions you're abstracting more. I think the function names are pretty self explanatory in most cases, e.g.: createPersonTable, correctDateDeath, getEndDate. Without reading the exact implementation I have a pretty good idea of what these do, from a non-maintainer perspective.
Lintr
Lintr picks up some warnings. The majority being no visible global function definition for 'x'.
Dependencies
methods only seems to be used for one call of getPackageName in checkInputs.R. purrr is only seems to be used for one call of map in checks.R. It is not worth to re-implement this, as purrr is a dbplyr dependency used by omopgenerics. And methods is a base dependency, installed by default.
In summary; With some clean up of the lintr warnings and more elaborate testing, it looks great!
On request of @catalamarti, here is a summary of the PaRe-report. I will (or have) send the entire report to Martí.
Function diagram
mockCdmFromTablesseems to kicks off a lot of functions, but is covered for 77%. Code coverage is obviously somewhat arbitrary, but considering the amount of stuff it does, I think bumping up the coverage would be nice.Code quality
The summary of the code quality seems great, no large functions, relatively simple functions. Some functions have quite some arguments (>7) but nothing too concerning. The largest function seems to be
mockCohort(args: 6, lines: 114, cycloComp: 8). But about a third of that is asserting the input arguments.Because of the smaller functions you're abstracting more. I think the function names are pretty self explanatory in most cases, e.g.:
createPersonTable,correctDateDeath,getEndDate. Without reading the exact implementation I have a pretty good idea of what these do, from a non-maintainer perspective.Lintr
Lintr picks up some warnings. The majority being
no visible global function definition for 'x'.Dependencies
methodsonly seems to be used for one call ofgetPackageNamein checkInputs.R.purrris only seems to be used for one call ofmapin checks.R. It is not worth to re-implement this, aspurrris adbplyrdependency used byomopgenerics. Andmethodsis a base dependency, installed by default.In summary; With some clean up of the lintr warnings and more elaborate testing, it looks great!