Skip to content

list-ops: reimplement ambiguous tests #1746

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 7 commits into from
Dec 24, 2020
Merged

list-ops: reimplement ambiguous tests #1746

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f19d893
Format
SleeplessByte Dec 18, 2020
4aaec16
Re-implement ambigious test cases
SleeplessByte Dec 18, 2020
f0712e1
Clarifying comment
SleeplessByte Dec 18, 2020
f4650f0
Re-add old versions of reimplemented tests
SleeplessByte Dec 18, 2020
8e08681
Remove accidantally commited executable
SleeplessByte Dec 19, 2020
b00f7aa
Clarifying commentary about integer division.
SleeplessByte Dec 23, 2020
ac1e44f
Add advice
SleeplessByte Dec 24, 2020
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
120 changes: 120 additions & 0 deletions exercises/list-ops/canonical-data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@
},
{
"description": "folds (reduces) the given list from the left with a function",
"comments": [
"For function: acc is the accumulator and el is the current element. ",
"The order of the arguments (ie. acc, el or el, acc) should match the ",
"conventions common to the language of the track implementing this. ",
"See https://github.com/exercism/problem-specifications/pull/1746#discussion_r548303995 for further advice."
],
"cases": [
{
"uuid": "613b20b7-1873-4070-a3a6-70ae5f50d7cc",
Expand All @@ -184,18 +190,78 @@
{
"uuid": "d2cf5644-aee1-4dfc-9b88-06896676fe27",
"description": "direction dependent function applied to non-empty list",
"comments": [
"Expects integer division (expects / to discard fractions). "
],
"property": "foldl",
"input": {
"list": [2, 5],
"initial": 5,
"function": "(x, y) -> x / y"
Copy link
Member

@petertseng petertseng Dec 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

note for posterity: The possible ambiguous interpretation for this function were:

  • acc, el -> acc / el
    • acc: 5, el: 2. new acc 2
    • acc: 2, el: 5. new acc 0
  • acc, el -> el / acc
    • acc: 5, el: 2. new acc 0
    • acc: 0, el: 5. division by zero.

So clearly the former was intended. Note that this is different from the other case, where the latter was intended. The fact that these two same strings require two different interpretations is what's confusing and is the motivation for this PR.

},
"expected": 0
},
{
"uuid": "36549237-f765-4a4c-bfd9-5d3a8f7b07d2",
"reimplements": "613b20b7-1873-4070-a3a6-70ae5f50d7cc",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input."
],
"description": "empty list",
"property": "foldl",
"input": {
"list": [],
"initial": 2,
"function": "(acc, el) -> el * acc"
},
"expected": 2
},
{
"uuid": "7a626a3c-03ec-42bc-9840-53f280e13067",
"reimplements": "e56df3eb-9405-416a-b13a-aabb4c3b5194",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input."
],
"description": "direction independent function applied to non-empty list",
"property": "foldl",
"input": {
"list": [1, 2, 3, 4],
"initial": 5,
"function": "(acc, el) -> el + acc"
},
"expected": 15
},
{
"uuid": "d7fcad99-e88e-40e1-a539-4c519681f390",
"reimplements": "d2cf5644-aee1-4dfc-9b88-06896676fe27",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input. Expects / to preserve fractions. Integer division",
"will not work here, since it would compute 1 / 24 = 0. Use the ",
"original test values (d2cf5644-aee1-4dfc-9b88-06896676fe27) if ",
"integer division is expected / required. "
],
"description": "direction dependent function applied to non-empty list",
"property": "foldl",
"input": {
"list": [1, 2, 3, 4],
"initial": 24,
"function": "(acc, el) -> el / acc"
},
"expected": 64
}
]
},
{
"description": "folds (reduces) the given list from the right with a function",
"comments": [
"For function: acc is the accumulator and el is the current element ",
"The order of the arguments (ie. acc, el or el, acc) should match the ",
"conventions common to the language of the track implementing this. ",
"See https://github.com/exercism/problem-specifications/pull/1746#discussion_r548303995 for further advice."
],
"cases": [
{
"uuid": "aeb576b9-118e-4a57-a451-db49fac20fdc",
Expand All @@ -222,13 +288,67 @@
{
"uuid": "be396a53-c074-4db3-8dd6-f7ed003cce7c",
"description": "direction dependent function applied to non-empty list",
"comments": [
"Expects integer division (expects / to discard fractions). "
],
"property": "foldr",
"input": {
"list": [2, 5],
"initial": 5,
"function": "(x, y) -> x / y"
Copy link
Member

Choose a reason for hiding this comment

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

note for posterity: The possible ambiguous interpretation for this function were:

  • acc, el -> acc / el
    • acc: 5, el: 5. new acc 1
    • acc: 1, el: 2. new acc 0
  • acc, el -> el / acc
    • acc: 5, el: 5. new acc 1
    • acc: 1, el: 2. new acc 2

So clearly the latter was intended. Note that this is different from the other case, where the former was intended. The fact that these two same strings require two different interpretations is what's confusing and is the motivation for this PR.

},
"expected": 2
},
{
"uuid": "17214edb-20ba-42fc-bda8-000a5ab525b0",
"reimplements": "aeb576b9-118e-4a57-a451-db49fac20fdc",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input."
],
"description": "empty list",
"property": "foldr",
"input": {
"list": [],
"initial": 2,
"function": "(acc, el) -> el * acc"
},
"expected": 2
},
{
"uuid": "e1c64db7-9253-4a3d-a7c4-5273b9e2a1bd",
"reimplements": "c4b64e58-313e-4c47-9c68-7764964efb8e",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input."
],
"description": "direction independent function applied to non-empty list",
"property": "foldr",
"input": {
"list": [1, 2, 3, 4],
"initial": 5,
"function": "(acc, el) -> el + acc"
},
"expected": 15
},
{
"uuid": "8066003b-f2ff-437e-9103-66e6df474844",
"reimplements": "be396a53-c074-4db3-8dd6-f7ed003cce7c",
"comments": [
"Reimplemented to remove ambiguity about the parameters of the ",
"function input. Expects / to preserve fractions. Integer division",
"will not work here, since it would compute 4 / 24 = 1 / 6. Use ",
"the original test values (be396a53-c074-4db3-8dd6-f7ed003cce7c) ",
"if integer division is expected / required."
],
"description": "direction dependent function applied to non-empty list",
"property": "foldr",
"input": {
"list": [1, 2, 3, 4],
"initial": 24,
"function": "(acc, el) -> el / acc"
Copy link
Member

@petertseng petertseng Dec 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The follow comment concerns notation only. It does not change the expected values of any test, or change the semantics of how foldr should operate.

I understand that some track maintainers may disagree about whether the function argument list should be represented as a string as (acc, el) or (el, acc), but I would like to say to those track maintainers that I think we are just going to have to pick one and run with it. Again, I emphasise that either way there would not be a change to the operation of foldr: The implementing track should be sure to pass the element as el and the accumulator as acc, no matter the string representation here.

I advise implementing tracks to be aware of the conventions common to their language do what makes sense for their language. Why do I bring this up? Observe:

  • Ramda (a functional library for Javascript): foldr is (a, b -> a) -> a -> [b] -> a. The passed function takes its accumulator first and the list element second, and foldr takes initial accumulator first and the list second.
  • OCaml List.fold_right is ('a -> 'b -> 'b) -> 'a list -> 'b -> 'b. The passed function takes its list element first and the accumulator second, and fold_right takes the list first and initial accumulator second.
  • Haskell Prelude foldr is class Foldable t where foldr :: (a -> b -> b) -> b -> t a -> b. The passed function takes its list element first and accumulator second, and foldr takes the initial accumulator first and the list second.

As you can see, I've listed three different languages doing things in three different ways, so I advise track maintainers looking to squabble over this that there is no universal convention, so we should just pick one for problem-specifications and run with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is great advice and I agree 100% with it. I'll link this comment in the comments in the canonical data.

},
"expected": 9
}
]
},
Expand Down