-
-
Notifications
You must be signed in to change notification settings - Fork 195
Add Explicit Args To Exercises - Part 3 #507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Explicit Args To Exercises - Part 3 #507
Conversation
03649b2
to
02f8a80
Compare
Unfortunately that means the canary we added in #454 died, and for the wrong reason.
Guess it would be better to remove it. At least http://x.exercism.io/v2/exercises/haskell/pov still works, eh? |
exercises/pov/src/POV.hs
Outdated
|
||
tracePathBetween :: Eq a => a -> a -> Tree a -> Maybe [a] | ||
tracePathBetween = error "You need to implement this function." | ||
tracePathBetween nodeA nodeB tree = error "You need to implement this function." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to make clear which node between the two given should be first in the path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fromPOV
we only know that the element may be in the Tree
, but node
suggests two things:
- It is part of a tree.
- It is a node, not a leaf.
I would suggest simply x
, because the meaning is already clear in the function's name.
In tracePathBetween
we have a similar problem but, as @petertseng mentioned, one is supposed to be the starting element and the other the ending element, so a think a pair of related names would be great. Some suggestions:
start
,end
from
,to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to make the changes. As the Haskell's POV page is broken, I checked the instructions for Ruby, which says:
This lets us more simply describe the paths between two nodes. So for example the path from 6-9 (which in the first graph goes up to the root and then down to a different leaf node) can be seen to follow the path 6-2-0-3-9
This exercise involves taking an input graph and re-orientating it from the point of view of one of the nodes.
Also, isn't a Leaf also considered a node (often called leaf node) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. A leaf is a node. 😄
@@ -1,7 +1,10 @@ | |||
module Triplet (isPythagorean, mkTriplet, pythagoreanTriplets) where | |||
|
|||
isPythagorean = error "You need to implement this function." | |||
isPythagorean :: (Int, Int, Int) -> Bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is done, then the reasoning in https://github.com/exercism/xhaskell/blob/master/exercises/pythagorean-triplet/.meta/DONT-TEST-STUB for why the tests would fail to build is false, so if we do this please delete that file so that the stub gets tested.
If we choose the type of mkTriplet
as (Int, Int, Int)
rather than letting the student choose, do we even need mkTriplet
? Shouldn't we just delete mkTriplet
then? I think the only point of it is to give the student the choice.
Why did we give this choice, anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we give this choice, anyway?
I didn't see explanation given in exercism/exercism#741. I guess we can see that mkTriplet
is invariant to ordering of the three numbers, since it will always order them in a consistent way.
So we still need mkTriplet
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can consider redesigning this exercise without mkTriplet
, @petertseng. The way it is now, I think it is over-specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I left this exercise for last because I couldn't figure out a way to declare the types without over-specifying it :/
I was completely unaware of https://github.com/exercism/xhaskell/blob/master/exercises/pythagorean-triplet/.meta/DONT-TEST-STUB though
What about get rid of this commit so this exercise can get redesigned as suggested by @rbasso ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should stop things here because of that. We don't have anything ready yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool. I've applied the feedback for the POV exercise, so I'm just waiting for the decision here.
02f8a80
to
cb571e0
Compare
Er? it works again? http://exercism.io/exercises/haskell/pov/readme Okay fine guess we can leave the canary in... |
Part of #473
The POV page is broken. I can open an Issue if that's the case.
Exercises covered: