Add vectorised unit test for Collatz Conjecture #111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sync fork with main repo
As a demonstration of the point I raised in issue exercism#109, here is an update for the non-core Collatz Conjecture exercise. The current version of this does not test for vector parameters and the example would fail this test. The following changes have been made: - An extra test has been added to the unit tests that send a four-element vector to the function. - In the example, he original function `collatz_step_counter` has been renamed to `collatz_scalar`. - Also in the example, the line `collatz_step_counter <- Vectorize(collatz_scalar)` has been added. This is a very minor change to the example, but makes the function a lot more powerful by taking full advantage of R's vectorisation capabilities. R developers really shouldn't be thinking in single values the way most mainstream programming languages do.
xarxziux
changed the title
|
I have this on the radar, and will try to review it this weekend. Sorry for the delay! |
| @@ -27,4 +27,9 @@ test_that("Negative input results in an error", { | |||
| expect_error(collatz_step_counter(-15)) | |||
| }) | |||
|
|
|||
| test_that("Answer can accept vector parameter", { | |||
There was a problem hiding this comment.
a) Is this message a clear enough to provide a learner (who progressed through our track without having to think about vectorising!) with sufficient guidance, so that they are able to refactor their most-tests-passing solution according to this last test?
b) Should we update the ReadMe as well? Maybe with a note that we start testing for compatibility with the R-idiomatic vector-support here?
As I mentioned in #109 (comment), I'm a novice when it comes to this topic.
katrinleinweber
approved these changes
Jan 17, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a demonstration of the point I raised in issue #109, here is an update for the non-core Collatz Conjecture exercise. The current version of this does not test for vector parameters and the example would fail this test.
The following changes have been made:
collatz_step_counterhas been renamed tocollatz_scalar.collatz_step_counter <- Vectorize(collatz_scalar)has been added.This is a very minor change to the example, but makes the function a lot more powerful by taking full advantage of R's vectorisation capabilities. R developers really shouldn't be thinking in single values the way most mainstream programming languages do.