Description
In the package we have an import dplyr and then importFrom dplyr . We should agree whether is it worth the effort to import from dplyr all the separate functions or not. In any case a clean up is necessary. The pros for having the importFrom are:
- documentation
- clear dependencies
The cons: - extra work
Additionally we should not import the %>% from dplyr, considering that we are importing %<>% from magrittr.
A similar cleanup is needed for the app part. In the server we have library(dplyr) and library(magrittr) despite using everywhere else the notation with ::. magrittr functions are used in only two places without the ::. dplyr is used more extensively for the data.frame manipulation for the plots.
The pros of using the :: notation:
- being consistent across the app
- do not have unnecessary dependencies
The cons: - extra effort
A separate but related discussion is given by the separation between package and app functionalities. In this view the data.frame manipulation for the plots should be moved out of the server and become a function in the package. On the other hand the function withModalSpinner.R should be moved into the server, as it belongs to the app and not the package.
Pros of having the withModalSpinner.R in the app:
- keep the separation between package and app
Pros of having thewithModalSpinner.Rin the package: - good documentation and examples.
Another option would be to create a small public package with shiny utilities, place the spinner there and load the package in SmaRP app.
In case of having thewithModalSpinner.Rin the package the documentation should be adapted accordingly (e.g. theshinyfunctionshowModalshould be imported.
Additional notes:
In the report there are library(reshape2) and library(kableExtra) without any reshape2 or kableExtra functions being used.