-
-
Notifications
You must be signed in to change notification settings - Fork 554
Correct the definition of triangle inequality to not include degenera… #375
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
Changes from 2 commits
450a25c
ddc9ced
e71f026
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,16 @@ The program should raise an error if the triangle cannot exist. | |
|
||
## Hint | ||
|
||
The sum of the lengths of any two sides of a triangle always exceeds or | ||
is equal to the length of the third side, a principle known as the _triangle | ||
In mathematics, a degenerate case is a limiting case in which an element of a | ||
class of objects is qualitatively different from the rest of the class and hence | ||
belongs to another, usually simpler, class. | ||
|
||
1. If the sum of the lengths of any two sides of a triangle is equal to the | ||
length of the third side, there exists a degenerate triangle. | ||
|
||
2. The sum of the lengths of any two sides of a triangle always exceeds or is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, there is a sentence about a degenerate case, then a numbered list, and the first item is a degenerate case, but the second isn't, and to me it seemed to make the relationship unclear (why are the two in a numbered list?). What if we start with saying the triangle inequality first, then explain the degenerate case of the inequality, as well as explaining what a degenerate triangle looks like? That would help us to understand what is meant by "qualitatively different from the rest of the class and hence belongs to another, usually simpler, class." by giving an example relevant to the exercise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of this? The triangle inequality theorem states: A corollary to the triangle inequality theorem is there are two classes of triangles. If the sum of the lengths of any two sides of a triangle is greater than the length of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a chance that it will be unclear to a reader that the two classes of triangles mentioned (not degenerate vs degenerate) are a separate classification from the one resulting from the lengths of the sides (equilateral, isosceles, scalene)? If there is, we should try to find a way to make the distinction clearer. But if others find it understandable, I think this is good. I think the last thing is: After a student has read this, the student might think "OK, I have just learned about degenerate triangles, and how should this knowledge apply to this exercise?" and we should give an answer to that potential question in the readme (that they don't violate the triangle inequality - but we won't test for them) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good points @petertseng. How about, "A corollary to the triangle inequality theorem is there are two classes of triangles--degenerate and nondegenerate. If the sum of the lengths of any two sides of a triangle is greater than the length of the third side (as is the case with equilateral, isosceles, and scalene triangles), that triangle is two dimensional, has area, and belongs to the nondegenerate class." Regarding your second point, knowledge about degenerate triangles applies to the exercise because, at least for the Ruby and Coffescript exercises, the second to last test is for the degenerate case. Interestingly enough I don't see a test for the degenerate case in the Javascript exercise. The main reason I wanted to point out degenerate triangles do not violate the triangle inequality theorem is the tests, at least for Ruby and Coffeescript, are currently wrong. The combination of incorrect tests and not understanding the triangle inequality theorem confused me. I thought either the tests were wrong or the definition of the triangle inequality theorem in the README was wrong. I incorrectly assumed the tests were right, hence the original pull request message regarding a correction to the definition of the triangle inequality theorem in the README. After a closer look I realized a 2,2,4 triangle satisfies the degenerate condition of the triangle inequality theorem, but In Coffeescript for example, the second to last test reads: it 'is illegal when violating triangle inequality 2', ->
expect(-> new Triangle(2,2,4)).toThrow("violation of triangle inequality") If it is a mistake that the Ruby and Coffeescript exercises are testing for the degenerate case, then I'm happy to make pull requests to remove those tests. Otherwise, I'm happy to fix the tests in the languages I know if that is ok with you all. If you choose not to test for the degenerate case, then a way to document in the README how this knowledge about degenerate triangles applies to the exercise would be to add a "Dig Deeper" or "Extra Credit" or "Challenge" section where you explain how none of the tests checked for a degenerate triangle, but you can challenge the reader to add those tests. You can even put the whole explanation of the corollary to the triangle inequality theorem in the "Challenge" section itself. For OO languages you can challenge the reader to prevent degenerate triangles from being initialized directly from the Triangle class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks, I think this is good. Explicitly noting them will prevent the confusion.
Ah. It appears that Javascript did have it at one point, but removed it for exercism/DEPRECATED.javascript#164 .
Ah, good point. Yes, the test should be removed altogether, as had been decided at #311 (comment) - tests for degenerate triangles are not in https://github.com/exercism/x-common/blob/master/exercises/triangle/canonical-data.json and tracks should be using this data, unless they have specific language-level concerns (and this likely isn't one).
I like this! I don't have a strong opinion on which of the three proposed terms to use. If you made me pick I'd pick "dig deeper", but I can always be convinced otherwise.
I do think this sounds like a good idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I was looking at the json here and you are not testing that triangles with negative sides are illegal. It seems like a good idea to test for that. The Ruby, Javascript and Coffeescript tests are currently testing for it. What do you think of this? HintThe triangle inequality theorem states: A corollary to the triangle inequality theorem is there are two classes of Dig DeeperThis exercise does not test for degenerate triangles. Feel free to add your own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah! It would be good to send a PR for that, then. I will first say that at #311 we had decided against it because it's not specified clearly whether the classification function is responsible for normalising possibly-negative distances into absolute lengths as opposed to the caller being responsible. If you do feel strongly about this, it is of course open to discussion if we could get a PR on it.
Personally I am thinking it looks good, so 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's interesting because it leaves it open to the student what to do with the degenerate triangles. You gave a possible option earlier in our discussion: It shouldn't be possible to create them because they are different. Others may choose to allow them, and classify them as either scalene or isosceles (they can't be equilateral unless all sides are zero). Others may make degenerate a separate case altogether. We like to see variety, so leaving it open seems like a good idea, which is another reason I said 👍 |
||
equal to the length of the third side, a principle known as the _triangle | ||
inequality_. | ||
|
||
Therefore, a degenerate triangle does not violate the principle of _triangle | ||
inequality_. |
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.
it might be better to make clear that, more than the degenerate triangle existing, it is the triangle referred to earlier in the sentence that is degenerate (something like "that triangle is degenerate")