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.

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

Refine wording in Saddle Points #2413

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Refine wording in Saddle Points #2413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
01a18b9
Refine wording
MatthijsBlom Mar 27, 2024
cd1ca0a
Update exercises/saddle-points/introduction.md
ErikSchierboom Apr 2, 2024
69e44c5
tallest and shortest wording
kotp Apr 5, 2024
2070187
fix typo
MatthijsBlom Apr 5, 2024
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
2 changes: 1 addition & 1 deletion exercises/saddle-points/instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Your task is to find the potential trees where you could build your tree house.
The data company provides the data as grids that show the heights of the trees.
The rows of the grid represent the east-west direction, and the columns represent the north-south direction.

An acceptable tree will be the largest in its row, while being the smallest in its column.
An acceptable tree will have the greatest height in its row, while having the least height in its column.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @jordancurve that there is a risk here of the very same confusion that we are trying to avoid.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't the largest or greatest height "tallest tree" and the least height being the "shortest tree", given the problem we are solving?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that «the tallest tree» might not be unique. People have been frustrated by test cases with several tallest/shortest trees per row/column, because based on the instructions they expected there to be only one.

Copy link
Member

Choose a reason for hiding this comment

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

The tests should enforce the detail though, right? So any ambiguity is clarified by expressed expectation, one would think.

Copy link
Member

Choose a reason for hiding this comment

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

The whole point of this PR is to clarify this point. It is okay fort the prose to omit a detail and for the test to enforce that detail. It is less okay for the prose to imply one thing and the test to enforce something that contradicts that thing.

Copy link
Member

Choose a reason for hiding this comment

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

The tests do seem to have covered multiple "shortest tree" at least in the single column test. Or to express it in the way the test explains it "saddle points" meaning there can be more than one.
I wholly agree that the prose should not contradict, while omission is OK (not great, necessarily, but sometimes purposefully done).

I think that the detail that there may be more than one saddle point is expressed in the tests even by name.

We may see that here:

    {
      "uuid": "ee99ccd2-a1f1-4283-ad39-f8c70f0cf594",
      "description": "Can identify that saddle points in a single column matrix are those with the minimum value",
      "property": "saddlePoints",
      "input": {
        "matrix": [
          [2],
          [1],
          [4],
          [1]
        ]
      },
      "expected": [
        {
          "row": 2,
          "column": 1
        },
        {
          "row": 4,
          "column": 1
        }
      ]
    },
    {
      "uuid": "63abf709-a84b-407f-a1b3-456638689713",
      "description": "Can identify that saddle points in a single row matrix are those with the maximum value",
      "property": "saddlePoints",
      "input": {
        "matrix": [
          [2, 5, 3, 5]
        ]
      },
      "expected": [
        {
          "row": 1,
          "column": 2
        },
        {
          "row": 1,
          "column": 4
        }

Copy link
Member

@kotp kotp Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
An acceptable tree will have the greatest height in its row, while having the least height in its column.
An acceptable tree will be the tallest in its row, while being the shortest in its column.

Copy link
Member

Choose a reason for hiding this comment

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

@kotp "will the tallest" should that be "will be the tallest"?

Copy link
Member

Choose a reason for hiding this comment

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

@MatthijsBlom What about the above suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This suggestion comes down to

-An acceptable tree will be the largest in its row, while being the smallest in its column.
+An acceptable tree will be the tallest in its row, while being the shortest in its column.

which doesn't solve the problem this PR aims to solve.

Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we use the phrasing from the introduction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could. I figured the difference between introduction.md and instructions.md might be deliberate, so I kept them different.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with unifying them, especially when it turns out that the terminology is so hard to get right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we use the same wording in both cases, we could also de-duplicate.

One potential advantage of using two different descriptions: these provide different perspectives on the same problem.

Copy link
Contributor Author

@MatthijsBlom MatthijsBlom Apr 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did not agree to applying this suggestion, but it was applied anyway.

The point of my change was to fix an inaccuracy. This now applied suggestion effectively reverts my change and reintroduces the original inaccuracy. This inaccuracy has previously irritated people, hence this PR. See the forum thread, and the other threads here.


A grid might not have any good trees at all.
Or it might have one, or even several.
Expand Down
4 changes: 2 additions & 2 deletions exercises/saddle-points/introduction.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ You need to analyze each grid on the map to find good trees for your tree house.

A good tree is both:

- taller than every tree to the east and west, so that you have the best possible view of the sunrises and sunsets.
- shorter than every tree to the north and south, to minimize the amount of tree climbing.
- no shorter than any tree to the east and west, so that you have the best possible view of the sunrises and sunsets.
- no taller than any tree to the north and south, to minimize the amount of tree climbing.
ErikSchierboom marked this conversation as resolved.
Show resolved Hide resolved
Loading