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: Add JSON test data #311

Merged
merged 1 commit into from
Aug 3, 2016
Merged

triangle: Add JSON test data #311

merged 1 commit into from
Aug 3, 2016

Conversation

petertseng
Copy link
Member

@petertseng petertseng commented Jul 31, 2016

Pursuant to dicussion in #202, we won't have cases where a + b = c.
Examples are (2, 4, 2, Isosceles) and (1, 3, 4, Scalene).

As of this writing, 21 tracks implemented triangle:

This JSON file contains the combination of all their tests.
Most tracks have the same set of tests.

Haskell has a case specific to itself added in
exercism/haskell#142
This case is added to the common set.

The Go and Nim tracks have tests with +/- Infinity and NaN.
These cases are not added.

Closes #264
Closes exercism/todo#157

"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:",
"See https://github.com/exercism/xgo/pull/208"
Copy link
Member Author

@petertseng petertseng Jul 31, 2016

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.

@kotp
Copy link
Member

kotp commented Aug 1, 2016

I just solved this in the Ruby track, and noticed that at first we test for 0's, then we test for "Triangle Equality" which provides the standard, simple, definition of a Triangle, mathematically. Once the tests for Triangle Equality happen, the tests for zero can go away.

As far as that goes, the Triangle Equality addresses the questions on negative numbers naturally as well, I think.

@rbasso
Copy link
Contributor

rbasso commented Aug 1, 2016

I agree with @petertseng. The function should receive the side's lengths, so they have to be positive.

I don't understand what would be the interpretation of a negative size, because a vector length cannot be negative:

https://en.wikipedia.org/wiki/Triangle_inequality

In Euclidean geometry and some other geometries, the triangle inequality is a theorem about distances, and it is written using vectors and vector lengths (norms):
∥ x + y ∥ ≤ ∥ x ∥ + ∥ y ∥

https://en.wikipedia.org/wiki/Norm_(mathematics)

In linear algebra, functional analysis, and related areas of mathematics, a norm is a function that assigns a strictly positive length or size to each vector in a vector space—save for the zero vector, which is assigned a length of zero. A seminorm, on the other hand, is allowed to assign zero length to some non-zero vectors (in addition to the zero vector).

@kotp
Copy link
Member

kotp commented Aug 1, 2016

Those symbols (The double pipes) indicate absolute values, correct? This indicates that a "negative" distance is simply a distance with perspective (or direction), the "length" is absolute. The choice to normalize or not can be made in the method/function or required to be normalized outside of said function. Which suggests that the values for those vectors could have been negative, but are normalized in that function. Hence my view that taking a negative value may not be inappropriate. But we are free to choose.

The formula you link to using absolute function on the given lengths, suggests that we could go either way, enforcing that we are using "absolute" values.

Given the links you provide and that you understand the formulas, I would challenge your feign of non-mathyness... to use a technical term.

@rbasso
Copy link
Contributor

rbasso commented Aug 1, 2016

The formula you link to using absolute function on the given lengths...

I think that this is where we disagree. I may be wrong but I think that - in that formula - x, y are vectors in a space.

... This indicates that a "negative" distance is simply a distance with perspective (or direction)...

Isn't signed real number just a 1-dimensional vector? If we treat the values in the exercise as 1-dimensional vectors, only degenerated triangles where a + b = c would be possible, right?

Given the links you provide and that you understand the formulas, I would challenge your feign of non-mathyness... to use a technical term.

I like math, but my knowledge is too shallow to allow me any confidence! 😄

@Insti
Copy link
Contributor

Insti commented Aug 1, 2016

The inspiration for this exercise is the triangle project from rubykoans.

The second part of that problem includes uses the following test cases:

assert_raise(TriangleError) do triangle(0, 0, 0) end
assert_raise(TriangleError) do triangle(3, 4, -5) end
assert_raise(TriangleError) do triangle(1, 1, 3) end
assert_raise(TriangleError) do triangle(2, 4, 2) end

I think, we should include degenerate triangles (above: 1, 3 & 4) in the test cases.
I think, we should not include triangles with negative edge lengths (above: 2).

@petertseng
Copy link
Member Author

I think, we should include degenerate triangles (above: 1, 3 & 4) in the test cases.

I do not care either way about (0, 0, 0) or the related case (10, 10, 0). I will gladly add (0, 0, 0) back in if all interested are agreed.

(1, 1, 3) is not a degenerate triangle; it is a violation of the triangle inequality. I say we have to include this one no matter what (and luckily it is already included).

I also do not care about (2, 4, 2), but many other people in #202 do. I only see myself as enacting the will of the people. I am happy to add it if that is what people want.
Corollary question: If we add (2, 4, 2), should we expect it to be illegal or isosceles? Note for historical context that we asked this question of all tracks in January and also recently merged #298.

@petertseng
Copy link
Member Author

petertseng commented Aug 1, 2016

I think, we should not include triangles with negative edge lengths (above: 2).

My current inclination is to keep a case such as (3, 4, -5). If we do not, we may see submitted solutions that would say (3, 4, -100) is a valid scalene triangle, if they were posed such a question.

I can be persuaded that we should not include such cases. For example, in #287 (comment) there was a concern about tests having edge-itis. If we think that's what's happening here, very glad to drop the test instead.

@kotp
Copy link
Member

kotp commented Aug 1, 2016

If 2, 4, 2 is degenerative it should be illegal. (and it is degenerative). If the area of the triangle becomes 0, it should fail.

The question on 3, 4, -5, then is do we pass or fail it. We should include it, it clarifies, but do we clarify in the negative or the positive?

When given a negative number, I can only assume that the length from 0 is 5... to me, it is a relative position based on 0. The length is definitely 5, even though it is a negative sign.

Pursuant to dicussion in #202, we won't have cases where a + b = c.
Examples are (2, 4, 2, Isosceles) and (1, 3, 4, Scalene).

Pursuant to discussion in #311, we'll defer (perhaps indefinitely) on
adding cases where one side length is negative.

As of this writing, 21 tracks implemented triangle:
* https://github.com/exercism/xcsharp/blob/master/exercises/triangle/TriangleTest.cs
* https://github.com/exercism/xcpp/blob/master/triangle/triangle_test.cpp
* https://github.com/exercism/xclojure/blob/master/exercises/triangle/test/triangle_test.clj
* https://github.com/exercism/xcoffeescript/blob/master/exercises/triangle/triangle_test.spec.coffee
* https://github.com/exercism/xlisp/blob/master/exercises/triangle/triangle-test.lisp
* https://github.com/exercism/xecmascript/blob/master/exercises/triangle/triangle.spec.js
* https://github.com/exercism/xelixir/blob/master/exercises/triangle/triangle_test.exs
* https://github.com/exercism/xelm/blob/master/exercises/triangle/TriangleTests.elm
* https://github.com/exercism/xerlang/blob/master/exercises/triangle/triangle_tests.erl
* https://github.com/exercism/xfsharp/blob/master/exercises/triangle/TriangleTest.fs
* https://github.com/exercism/xgo/blob/master/exercises/triangle/triangle_test.go
* https://github.com/exercism/xhaskell/blob/master/exercises/triangle/test/Tests.hs
* https://github.com/exercism/xjava/blob/master/exercises/triangle/src/test/java/TriangleTest.java
* https://github.com/exercism/xjavascript/blob/master/exercises/triangle/triangle.spec.js
* https://github.com/exercism/xlua/blob/master/exercises/triangle/triangle_spec.lua
* https://github.com/exercism/xnim/blob/master/triangle/triangle_test.nim
* https://github.com/exercism/xperl5/blob/master/triangle/cases.json
* https://github.com/exercism/xpython/blob/master/exercises/triangle/triangle_test.py
* https://github.com/exercism/xruby/blob/master/exercises/triangle/triangle_test.rb
* https://github.com/exercism/xscala/blob/master/exercises/triangle/src/test/scala/triangle_test.scala
* https://github.com/exercism/xswift/blob/master/exercises/triangle/TriangleTest.swift

This JSON file contains the combination of all their tests.
Most tracks have the same set of tests.

Haskell has a case specific to itself added in
exercism/haskell#142
This case is added to the common set.

The Go and Nim tracks have tests with +/- Infinity and NaN.
These cases are not added.

Closes #264
Closes https://github.com/exercism/todo/issues/157
@petertseng
Copy link
Member Author

petertseng commented Aug 1, 2016

Time to decide!

Here is my understanding of all test cases that require a decision, and what I have done with them in the current state of this PR.

  • Exclude means a given case is not tested.
  • Include, Illegal means a given case is tested, and the test expects that it is an illegal triangle.
  • Include, Legal is the same as above, but s/illegal/legal/.

However, one thing I will do is look through the existing tracks and see how they treat such cases.

Guess Go's the outlier, and that's my doing too. Nevertheless, that's moot if the case is to be removed.

@kytrinyx
Copy link
Member

kytrinyx commented Aug 1, 2016

(3, 4, -5): Exclude

I would vote to Exclude, and support the suggestion that if it gets included later we should update the README.

(2, 4, 2), (1, 3, 4), etc.: Exclude

I would also vote to Exclude here, with the reasoning being that the difference between "not a triangle" and "triangle but degenerate" is more about the math than about the interesting part of the programming challenge. I do tend to get pulled into "yes but it's WRONG" discussions, because I am a hopeless pedant (at least when I've had enough sleep to pay attention to the discussion), but I think that it is not helpful to the exercise itself to focus on this issue.

@petertseng petertseng merged commit 81a9c46 into exercism:master Aug 3, 2016
@petertseng petertseng deleted the triangle-json branch August 3, 2016 01:54
@petertseng
Copy link
Member Author

Your choice is made!

Thanks, everyone.

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

Successfully merging this pull request may close these issues.

5 participants