search

carpentries-incubator / targets-workshop

Pre-alpha {targets} workshop
https://carpentries-incubator.github.io/targets-workshop/
Other
33 stars 6 forks source link

Function #14

Open drmowinckels opened 1 year ago

github-actions[bot] commented 1 year ago

:ok: Pre-flight checks passed :smiley:

This pull request has been checked and contains no modified workflow files or spoofing.

It should be safe to Approve and Run the workflows that need maintainer approval.

joelnitta commented 1 year ago

Thanks @drmowinckels !

I just have one concern with the PR: I'm not sure it's a good idea to use the Extract Function RStudio tool. On one hand, I can see how it demonstrates conversion of non-function code to a function. But on the other hand, it adds cognitive load because the learner has to grasp a new tool. If it worked without further modification I would probably say it's worth it. But it requires additional fiddling, hence additional cognitive load (explanation of NSE). So I think it might be better to leave it out and just focus on manual conversion to a function. Extract function actually strikes me as a tool for advanced users who probably can already write their own functions. It is a shortcut that speeds things up a bit, but you still have to know what you are doing to use it.

drmowinckels commented 1 year ago

That's an excellent point! Its actually really good tool when dealing with stuff that has standard evaluation, but for the example we have, indeed it ends up being more to explain than necessary.

I'll omit that part.

drmowinckels commented 1 year ago

I added some info about the tool in a callout. I couldnt get it rendering locally (some os lib issues I need to fix). But I thought it was worth having a look at it like that, and then delete if it still looks confusing.