Skip to content

triangle: Add JSON test data #311

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 3, 2016
Merged

triangle: Add JSON test data #311

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions triangle.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
"#": [
"Pursuant to discussion in #202,",
"we have decided NOT to test triangles where all side lengths are positive but a + b = c.",
"e.g: (2, 4, 2, Isosceles), (1, 3, 4, Scalene).",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[10, 10, 0] and [0, 0, 0] are both triangles where a + b = c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reductio ad absurdum: If we're not including degenerate triangles, these tests should not be present either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since 10, 10, 0 is a line and 0, 0, 0 is a point, and the discussion happened where we are not including the fringe of degenerate triangles, they should not be here.

Copy link
Member

@kotp kotp Jul 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are testing for failure, not for inclusion. so I think it is good.

Copy link
Member Author

@petertseng petertseng Jul 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible course of action may be to clarify "we have decided NOT to test triangles where a, b, c are all positive and a + b = c" because as I see it, the point of (0, 0, 0, Illegal) and (10, 10, 0, Illegal) are to test that 0 is not a valid side length, not just to be a case like (2, 4, 2, Isosceles) or (1, 3, 4, Scalene).

However, it may be the case that cases involving zero should also get thrown out for the same reasons as in #202: they raise questions about degeneracy. Can we call it a triangle even if some side lengths are zero? And unrelated to this exercise but related to the previous question: Can we consider every triangle to be a quadrilateral with exactly one side length zero? I don't have a definitive answer.

  • If we can find a definitive reason to reject 0 as a valid side length, (0, 0, 0, Illegal) and (10, 10, 0, Illegal) should stay and the above sentence in the file should be amended.
  • If we cannot, the cases should go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had already found a definitive reason in prior discussions, regarding degenerative triangles. Doesn't 0 as a any length create a degenerative triangle, and therefore is rejected on that basis?

I think we have decided to test, and decided to fail them, so the sentence there does not make sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(0, 0, 0, Illegal) and (10, 10, 0, Illegal) are hereby removed.

"It's true that the triangle inequality admits such triangles.",
"These triangles have zero area, however.",
"They're degenerate triangles with all three vertices collinear.",
"(In contrast, we will test (0, 0, 0, Illegal), as it is a point).",

"The expectation are given as strings,",
"but your language may use the appropriate representations.",
"For example, enums, variants, or tagged unions all are viable candidates.",

"Your track may choose to have the 'illegal' result be another member of the enum/variant/union/etc.,",
"or instead to signal an error/exception/etc. on an illegal triangle.",

"If appropriate for your track, you'll need to ensure that no pair of expected values are equal.",
"Otherwise, an implementation that always returns a constant value may falsely pass the tests.",
"See https://github.com/exercism/xgo/pull/208"
Copy link
Member Author

@petertseng petertseng Jul 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could have sworn there was another, but now I can't find it

Found it. The other instance was sublist, not triangle. Commenting on new diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also applicable to sublist because exercism/python#342 - maybe I'll add a comment to that json file

],
"cases": [
{
"description": "equilateral triangle has all sides equal",
"sides": [2, 2, 2],
"expected": "equilateral"
},
{
"description": "larger equilateral triangle",
"sides": [10, 10, 10],
"expected": "equilateral"
},
{
"description": "isosceles triangle with last two sides equal",
"sides": [3, 4, 4],
"expected": "isosceles"
},
{
"description": "isosceles triangle with first two sides equal",
"sides": [4, 4, 3],
"expected": "isosceles"
},
{
"description": "isosceles triangle with first and last sides equal",
"sides": [4, 3, 4],
"expected": "isosceles"
},
{
"description": "isosceles triangle with unequal side larger than equal sides",
"sides": [4, 7, 4],
"expected": "isosceles"
},
{
"description": "scalene triangle has no equal sides",
"sides": [3, 4, 5],
"expected": "scalene"
},
{
"description": "larger scalene triangle",
"sides": [10, 11, 12],
"expected": "scalene"
},
{
"description": "scalene triangle with sides in descending order",
"sides": [5, 4, 2],
"expected": "scalene"
},
{
"#": "Your track may choose to skip this test and deal only with integers if appropriate",
"description": "small scalene triangle with floating point values",
"sides": [0.4, 0.6, 0.3],
"expected": "scalene"
},
{
"description": "a triangle violating the triangle inequality is illegal",
"sides": [7, 3, 2],
"expected": "illegal"
},
{
"description": "two sides equal, but still violates triangle inequality",
"sides": [1, 1, 3],
"expected": "illegal"
},
{
"description": "triangles with all sides zero are illegal",
"sides": [0, 0, 0],
"expected": "illegal"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should negative size lengths be illegal? They should not survive as negative, but perhaps be normalized to positive numbers?

Copy link
Member Author

@petertseng petertseng Jul 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you strongly advocate for this? If you do, I would say that accompanying this case should be also cases, both legal and illegal, with two and three negative side lengths, and covering all triangle types as well. (2, 4, -7, Illegal), (2, -2, -2, Equilateral), (-7, 7, 4, Isosceles), etc.

I argue against, because I say the function under test should be considered to receive side lengths after normalisation and so it cannot receive a negative value.

But my actual main reason for opposition is simply pragmatic instead of grounded in solid mathematical foundation: I don't want to be the one coming up with all the cases. Of course, that problem can be solved in the obvious fashion (one interested in negative inputs provides all the cases).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one test that given any given valid (negative) number in any position would suffice. Normalize all values to the 'absolute' value. We don't, after all, test against a coordinate plane, (or, rather, a location in space) so rotation in this case shouldn't matter.

What you are saying is that the user of the Triangle class should decide to normalize or not, in which case, the burden of testing is only for positive lengths, which is also fine.

I say, then, match it to the readme, and let the discussion ensue on the exercism site.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one test that given any given valid (negative) number in any position would suffice.

If we decide that we should test negative values, I would argue for at least adding the three cases I describe. We wouldn't want someone to consider (2, -2, -2) an isosceles triangle rather than equilateral, after all.

I say, then, match it to the readme

And now I have to awkwardly announce that it is not noted in the README. We can fix that, of course!!!

]
}