Added additional challenge to 04-cond to combine conditional.

[jhollist]

@jdblischak and I discussed adding an additional challenge to this exercise. I have added one below that built on the plot_dist() challenge. It has the students combine logical operators (i.e. using &).

As this is my first challenge, I would be interested in any feedback.

[jdblischak]

Thanks, @jhollist. Overall it looks good. Here are a few suggestions:

• In the novice R lessons, we have chosen to always use the term "argument" instead of "parameter". So for the sake of consistency please edit to use "argument".
• Since you want the boolean argument to be a default argument, you should explicitly mention this.
• Having boxplot represent both a boolean value and a function is a little confusing. Could you think of a different name for the argument?

[jhollist]

@jdblischak good points! Just updated the PR.

[jdblischak]

I realized that we don't use the term "boolean" in these lessons. Instead could you say something like, "In order to choose between a histogram and a boxplot, we'll add an additional argument, use_boxplot. By default we will set use_boxplot to TRUE, which will create a boxplot when the vector is longer than threshold. When set to FALSE, plot_dist will instead plot a histogram when the vector is longer than threshold." Basically, refer to the name of the argument itself, use_boxplot, instead of referring to it as a boolean argument.

[jhollist]

@jdblischak done and good catch! I have trouble keeping terminology consistent when it is just myself.

[jdblischak]

Looks good. Thanks.

And just some general advice, it is often a better idea to make your changes on a feature branch for a PR. It does not matter much in this case because the PR was merged so quickly. But if you submitted a larger PR and it took longer to review, you would want to be able to continue pushing changes to your feature branch to update the PR but also keeping the master branch up-to-date for other development. From the GitHub docs (they use the term topic branch):

Pull requests can be sent from any branch or commit but it's recommended that a topic branch be used so that follow-up commits can be pushed to update the pull request if necessary.

[jhollist]

Thanks for the advice on the branches. I use git pretty extensively these days, but mostly for my own work. The SWC stuff is helping me figure out the workflow in a collaborative setting which is nice.

If you need other reviews or contributions let me know. I may be able to carve out some time to help.

On Fri, Oct 3, 2014 at 5:37 PM, John Blischak notifications@github.com wrote:

Merged #779 https://github.com/swcarpentry/bc/pull/779.

— Reply to this email directly or view it on GitHub https://github.com/swcarpentry/bc/pull/779#event-174039781.

Jeff W. Hollister email: jeff.w.hollister@gmail.com google voice: 401 326 2531 cell: 401 556 4087