darts: Add new exercise #498
Conversation
There was a problem hiding this comment.
@ramadis Thanks very much for adding this new exercise!
I have a couple of change requests, primarily stylistic. Also, you will need to update the config to add your new exercise. I don't think the build server actually ran your code, which might explain why the linter didn't complain about the module's main signature.
Please let me know if you have any questions or if I can help further.
exercises/target/example.js
Outdated
| @@ -0,0 +1,17 @@ | |||
| export default function solve(x, y) { | |||
| // Define as numbers | |||
| x = Number(x); | |||
There was a problem hiding this comment.
I think we should avoid re-assigning the input params. I'm surprised the linter didn't complain about this. We're currently using the Airbnb style guide: http://airbnb.io/javascript/#functions--mutate-params
exercises/target/target.spec.js
Outdated
| expect(solve(x, y)).toEqual(expected); | ||
| }); | ||
|
|
||
| test('A dart lands just in the border of the target', () => { |
There was a problem hiding this comment.
All tests but the first should be disabled with xtest rather than test.
exercises/target/example.js
Outdated
| y = Number(y); | ||
|
|
||
| // Check for NaN | ||
| if (x !== x || y !== y) return null; |
There was a problem hiding this comment.
Let's be more semantic and validate as early as possible, before the Number() calls. Actually using isNaN will mean we don't need the comment here:
if (isNaN(x) || isNaN(y)) return null;
|
@ramadis did you need any help making these changes, or did you have questions or concerns about them? Let me know where you're at. Thanks! |
|
@matthewmorgan I was actually wrapping up some details on the problem specification, so I can update all the fixes that were made there. I hope they approve that PR this week so I can update this one! |
|
@matthewmorgan could you help me with the error thrown by travis? It says Edit: I also run |
|
@ramadis when I pull your branch and run Closer inspection reveals that you have a mismatch in your git diff
diff --git a/exercises/darts/package.json b/exercises/darts/package.json
index f625e85..af94361 100644
--- a/exercises/darts/package.json
+++ b/exercises/darts/package.json
@@ -12,7 +12,7 @@
"babel-eslint": "^10.0.1",
"babel-jest": "^21.2.0",
"babel-plugin-transform-builtin-extend": "^1.1.2",
- "babel-preset-env": "^1.4.0",
+ "babel-preset-env": "^1.7.0",
"eslint": "^5.6.0",
"eslint-config-airbnb-base": "^13.1.0",
"eslint-plugin-import": "^2.14.0",You'll need to commit that change and push to this branch again. |
|
Thanks @ramadis ! |
Implementation in javascript of the exercise
darts. PR in problem-specification here: exercism/problem-specifications#1363