Skip to content
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

triangle: incorrect test in some tracks #85

Closed
kytrinyx opened this issue Jan 29, 2016 · 17 comments
Closed

triangle: incorrect test in some tracks #85

kytrinyx opened this issue Jan 29, 2016 · 17 comments
Assignees

Comments

@kytrinyx
Copy link
Member

Please check if there's a test that states that a triangle with sides 2, 4, 2 is invalid. The triangle inequality states that for any triangle, the sum of the lengths of any two sides must be greater than or equal to the length of the remaining side. If this doesn't affect this track, go ahead and just close the issue.

@NobbZ
Copy link
Member

NobbZ commented Jan 29, 2016

There is such a test which demands a failure. AFAIR I just took this from JSON or another track. Is it enough to just remove that test or shall I rewrite it to test for a specific kind?

I will tackle this after #84 is finished and merged.

@NobbZ NobbZ self-assigned this Jan 29, 2016
@kytrinyx
Copy link
Member Author

Oh, good question. I think we should have it return an isosceles triangle -- it's a good edge case to have.

@NobbZ
Copy link
Member

NobbZ commented Jan 30, 2016

Hmm, I just asked Wolfram alpha, it does classify as degenerated.

So I'd go for either taking further research when a triangle is degenerated
exactly and add it as another possible return value for kind. Or leaving as
is (error) or do classify as isosceles, because this is what comes when you
simply change the inequality.

Since we have 1 o'clock in the morning here, I'll go to bed now, but I'll
do some more research tomorrow and open a ticket in xcommon (or chime into
an ongoing one if there is one).

Katrina Owen notifications@github.com schrieb am Sa., 30. Jan. 2016 00:58:

Oh, good question. I think we should have it return an isosceles triangle
-- it's a good edge case to have.


Reply to this email directly or view it on GitHub
#85 (comment).

@masters3d
Copy link

This is very interesting. You are right.
2,4,2 is not a triangle because since 2 + 2 equals the length of the 4, then the 2 + 2 sides merge into the same plane so you end up with only a line.. (Technically is is three lines that occupy the same space of the 4 length)

@kytrinyx
Copy link
Member Author

So I'd go for either taking further research when a triangle is degenerated exactly and add it as another possible return value for kind.

So wikipedia is wrong, huh. Imagine that! OK, let's do some more research.

@hankturowski
Copy link

Wikipedia is not wrong. The 2,2,4 is a degenerate triangle, but still a triangle. In either case, we should call out our definition clearly in the .md.

@behrtam
Copy link

behrtam commented Jan 30, 2016

So for a degenerate triangle (e.g. 2,2,4) which looks like a line segment do we return "isosceles" as kind or "degenerate"?

@hankturowski
Copy link

My opinion is that we either add degenerate as a case, or specifically say in our problem definition that degenerate triangles should be rejected.

To be honest, rejecting degenerate triangles feels like a cleaner solution. I'd just like the problem description to get an update.

@behrtam
Copy link

behrtam commented Jan 30, 2016

triangle.md could really need some improvement. I think it should also mention the 3 kinds.

+1 rejecting degenerate triangles

@masters3d
Copy link

++ for adding degenerate as a case

We can then separate the math definition from the notion of accepting or rejecting as a triangle, thus moving the conversation away from declaring what is or not a triangle, to just stating the type of case.
(I personally think degenerate triangles are not triangles and should be rejected! they are just lines! ;) "the emperor has no clothes" yo! ).

@yurrriq
Copy link
Member

yurrriq commented Jan 30, 2016

It seems like the general vibe here is to reject degenerate triangles and update the .md accordingly, yeah?

@yurrriq
Copy link
Member

yurrriq commented Jan 30, 2016

👍

@NobbZ
Copy link
Member

NobbZ commented Jan 30, 2016

So actually, we leave all the "faulty" tests as they are and repair the
"correct" ones?

Which again would mean I don't need to do anything here?

Katrina Owen notifications@github.com schrieb am Sa., 30. Jan. 2016 20:25:

yeah, agreed


Reply to this email directly or view it on GitHub
#85 (comment).

@yurrriq
Copy link
Member

yurrriq commented Jan 30, 2016

I'm not sure I understand your double quotes, @NobbZ, but the xerlang triangle tests look good as they are.

yurrriq added a commit to exercism/clojure that referenced this issue Jan 30, 2016
- Fold invalid-sides into type
- Simplify the illogical check by removing the unnecessary (<= a 0)
- sort abc in descending order and don't rebind a, b and c
- Be explicit about how three distinct sides makes a scalene triangle
- Add link in triangle_test.clj to discussion re: degenerate triangles

Note: This closely mirrors the xlisp example now.
https://github.com/exercism/xlisp/blob/master/triangle/example.lisp

Close #95 (again) per discussion:
exercism/erlang#85
@kotp
Copy link
Member

kotp commented Jan 30, 2016

I say that we do exclude examples that stretch the definition to an absurd[1] degree. We keep to general position not special position geometry, and keep this where it should be in the context of the program here. If the assumption is that Triangles have area, this goes against that assumption, and is suitable for our purposes, and keeps everything at a non-college level, and is reasonable.

We don't test for the other more commonly known triangles, which is to say "right, acute or obtuse" why are we jumping into special position before we talk about all the general position triangles?

This is (as far as I can see) meant to be a simple exercise, even more so since we test for only 3 specific triangles.

@kytrinyx
Copy link
Member Author

Yeah, having slept on this I think that the right approach is to avoid the edge cases and stick to the basics (having things that obviously violate the triangle inequality thing is great, the distinction between not-a-triangle and yes-but-degenerate seems like a tangent that isn't useful here.

@kytrinyx
Copy link
Member Author

I've summarized this issue here: exercism/problem-specifications#202

I am closing this issue. If necessary I will open a new issue once we have come to an agreement about how to resolve the inconsistencies and what exact steps to take.

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

No branches or pull requests

7 participants