Skip to content

Simplify assess_bathing_quality_eu()#4

Open
hsonne wants to merge 2 commits intomasterfrom
quality
Open

Simplify assess_bathing_quality_eu()#4
hsonne wants to merge 2 commits intomasterfrom
quality

Conversation

@hsonne
Copy link
Copy Markdown
Member

@hsonne hsonne commented Feb 23, 2019

Hello @ca-vi, is this your function? I simplified it and would like to merge the change. Do you agree, author?

I applied the DRY principle (don't repeat yourself) by just assigning a number between 1 and 4 to index and then using this index in the last line of the function that is the only line now that creates a factor. I use factor(..., ordered = TRUE) instead of ordered() because I read that ordered() is just there for "S compability".

- swap sides of comparison: "mean > 15" intead of
  "15 < mean"
- do not repeat "return(ordered(..., levels = levels)"
@hsonne hsonne requested a review from ca-vi February 23, 2019 09:49
@ca-vi
Copy link
Copy Markdown
Collaborator

ca-vi commented Feb 28, 2019

Yes I am the author ;)

Your changes look fine to me! Please be sure to test with low number of e.coli values (only 2 or even only 1 value) and maybe in combination with a data.frame and sapply (or purrr::map for reasons unknown). I cannot remember if I have written any tests regarding this function...

@mrustl mrustl modified the milestone: v0.2.2 Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants