search

nlmixrdevelopment / nlmixr

nlmixr: an R package for population PKPD modeling
https://nlmixrdevelopment.github.io/nlmixr/
GNU General Public License v2.0
115 stars 45 forks source link

Break ui.R into smaller parts? #516

Closed billdenney closed 3 years ago

billdenney commented 3 years ago

As I was trying to work through #514, I started diving into ui.R again. It's pretty big at 149 kB of code, and it takes some work to mentally move through.

Would you accept a PR that did two things to ui.R:

  1. Separate it into a few different files (based on number of kB of code, I'd guess around 10 files would be the target)
  2. Unnest functions so that no (or very few) functions were defined inside of other functions
mattfidler commented 3 years ago

Thank you for the offer. However, probably not.

While I agree with the idea, the effort is already underway as it transitions to RxODE, so it would seem like wasted effort with the possibility of regressions that I would need to fix instead of working on the refactoring.

If you wish, you can see the progress, and also see that the functions that I am using are much smaller and easier to track if you look at the new branch. However the amount of code could still be considered large:

https://github.com/nlmixrdevelopment/RxODE/blob/muref/R/mu.R

There are also some additional features I'm working through for saem in this UI refactoring.