Merck / gsDesign2

Group Sequential Design Under Non-Proportional Hazards
https://merck.github.io/gsDesign2/
GNU General Public License v3.0
19 stars 8 forks source link

Tweaking to_integer() #448

Closed yihui closed 1 month ago

yihui commented 1 month ago

Removed the large amount of code repetition and the unnecessary for-loops. Please see individual commit messages if you find it difficult to understand the full changes.

I did this only because "while I'm looking at the code here..." :)

If my performance is evaluated based on the number of lines of code deleted rather than added, I can probably do better :D I compared the ahr and wlr cases carefully by eyes, and it will be helpful if someone else could give it another pass to make sure I didn't miss other differences in the code for the two cases. First let's see if the PR would pass R CMD check.

yihui commented 1 month ago

@LittleBeannie You are welcome! Just FYI, do.call() can be handy sometimes, but it does have one drawback, i.e., when errors occur, the error message and traceback won't be as clear as when you call the function directly (there's no function name in the traceback).

f = function() stop('No')

f()
#> Error in f() : No

do.call(f, list())
#> Error in (function ()  : No

So normally I'd avoid using do.call() unless I need to call functions dynamically (e.g., conditionally) or call a function with dynamic arguments.

LittleBeannie commented 1 month ago

@LittleBeannie You are welcome! Just FYI, do.call() can be handy sometimes, but it does have one drawback, i.e., when errors occur, the error message and traceback won't be as clear as when you call the function directly (there's no function name in the traceback).

f = function() stop('No')

f()
#> Error in f() : No

do.call(f, list())
#> Error in (function ()  : No

So normally I'd avoid using do.call() unless I need to call functions dynamically (e.g., conditionally) or call a function with dynamic arguments.

Gained another valuable lesson, and thank you for sharing your experience with do.call().