exercism / problem-specifications

Shared metadata for exercism exercises.
MIT License
326 stars 541 forks source link

saddle-points: is the order of expected pairs indicative of the order in which they should be found? #1294

Closed rpottsoh closed 5 years ago

rpottsoh commented 6 years ago

Moving conversation that began here.

While implementing the changes introduced in #1288 I discovered that a case had expected coordinates listed in descending order. All of the other cases with multiple pairs of coordinates are listed in ascending order.

My opening question to @Alexhans: @Alexhans is there significance to the order of expected coordinates in case Can identify saddle points in a non square matrix? I am working on an example solution which computes the correct coordinates for the two saddle points. However, the points found are in (0,0), (0,2) order, opposite as you have indicated in the testdata.

For the time being I am going to assume the order is not important. If it isn't then a comment or some other modification should be made to the testdata.

@cmccandless stated: If the order of the points is actually relevant, the list of points ought to be sorted (probably in ascending order).

To which I replied: The order of the coordinate pairs is opposite that of the other tests that have more than one saddle-point. There is nothing stated in the README or in the testcase about order. To me the order shouldn't be relevant. I think in most cases a solution is going to start at (0,0) when looking for valid points.

As long as the number of actual points agrees with the number of expected points and each actual point matches each expected point, that should be enough.

A consensus was reached to open this issue....

Alexhans commented 6 years ago

@rpottsoh I don't think the order of the saddle points should be relevant for an answer. I think of it more in terms of set equality.

rpottsoh commented 6 years ago

So should there be a PR to sort the expected coordinates into ascending order for Can identify saddle points in a non square matrix? For me the confusion lies with is the expected result an ordered set or not?

with input matrix of:

          [3, 1, 3],
          [3, 2, 4]

My solution finds:

        {
          "row": 0,
          "column": 0
        },
        {
          "row": 0,
          "column": 2
        }

while the testdata expects:

        {
          "row": 0,
          "column": 2
        },
        {
          "row": 0,
          "column": 0
        }

I assumed this was the default, specially because one thinks of a set of points and there's no indication otherwise. I'm not against a clarification, though, since I don't think it hurts.

Since the specifications are not clear on this issue, it is left to the track maintainers to decide how the actuals and expecteds are checked for equality. Since the pattern has been ascending order it is easiest to simply assert that expected and actual are equal. Expected and Actual are usually an array of tuples. The check ends up being 1:1 from beginning to end, so order becomes relevant.

petertseng commented 6 years ago

Whatever decision is made, I recommend to apply it to other exercises where the answer is a set (unordered), but represented in JSON as array. For example, anagram, allergies, go-counting, palindrome-products. Please do not assume that list is complete.

I recommend that readers of the file should understand that the items are unordered and thus to make no assumptions about any particular order, but I don't have a recommendation about how to promote awareness of this expected understanding, or whether it is necessary to do so.

rpottsoh commented 6 years ago

It would probably be best to note it in the testdata of those exercises where the order is important and isn't obvious as so. I can't think of any exercises off the top of my head, at the moment that fit that description, especially since I now know that the order for saddle-points is not important. :smile:

I am not sure if a policy should be created or it be otherwise documented regarding this issue.

Otherwise this issue can be closed I think.