Can error handling be improved here?

[Anton-4]

From https://github.com/exercism/roc/pull/118 :

My solution is really verbose. Sometimes I don't really know how to solve a simple problem without a full page of code in Roc, with a lot of error handling. I mean here's the equivalent code in Python, it took me about 2 minutes to write. I'm not quite there yet in Roc:

matrix = [[1,5,3],[1,4,3],[1, 2, 2]]
result = [{"row": y + 1, "column": x + 1} for (y, row) in enumerate(matrix) for (x, val) in enumerate(row)
if val == max(row) and val == min((0 if len(r) < x else r[x]) for r in matrix)]

[ageron]

Thanks for filing this issue @Anton-4 . Here's the corresponding Roc code. It's about 30 lines of code, but it's probably possible to reduce that a bit.

module [saddlePoints]

Forest : List (List U8)
Position : { row : U64, column : U64 }

saddlePoints : Forest -> Set Position
saddlePoints = \treeHeights ->
    tallestTreesEastWest =
        treeHeights
        |> List.mapWithIndex \row, rowIndex ->
            maxInRow = row |> List.max |> Result.withDefault 0
            row
            |> List.mapWithIndex \height, columnIndex ->
                if height == maxInRow then [{ row: rowIndex + 1, column: columnIndex + 1 }] else []
            |> List.joinMap \id -> id
        |> List.joinMap \id -> id
        |> Set.fromList

    numColumns = treeHeights |> List.map List.len |> List.max |> Result.withDefault 0
    smallestTreeNorthSouth =
        List.range { start: At 0, end: Before numColumns }
            |> List.map \columnIndex ->
                column =
                    treeHeights
                        |> List.mapWithIndex \row, rowIndex ->
                            row
                                |> List.get? columnIndex
                                |> \height -> Ok { height, rowIndex }
                        |> List.keepOks \id -> id

                minInColumn = column |> List.map .height |> List.min |> Result.withDefault 0
                column
                |> List.keepIf \{ height } -> height == minInColumn
                |> List.map \{ rowIndex } -> { row: rowIndex + 1, column: columnIndex + 1 }
            |> List.joinMap \id -> id
            |> Set.fromList

    tallestTreesEastWest |> Set.intersection smallestTreeNorthSouth

Of course, conciseness must not hinder clarity and readability. My one-liner above is hard to read, but if you space it out just a bit (about 6 to 8 lines), it becomes very readable and intuitive, imho:

def saddle_points(tree_heights):
    row_maxs = [max(row) for row in tree_heights]
    col_mins = [min(col) for col in zip(*tree_heights)]
    return [
        {"row": row_index + 1, "column": col_index + 1}
        for (row_index, row) in enumerate(tree_heights)
        for (col_index, val) in enumerate(row)
        if val == row_maxs[row_index] and val == col_mins[col_index]
    ]

I looked at the solutions of other languages, and Ruby is about as concise. Rust is around 15 lines. Several other languages are around 15-20 lines long. My Roc solution is 30 lines long, but perhaps someone can do better? The solution for the Java track is over 80 lines long, split across two files. And the Go solution is over 150 lines long (huh?!).

I really struggled to write concise code on this particular exercise, so think it's a good case study to see what can be done improve conciseness. Perhaps it's the error handling. Perhaps it's the lack of a List.mapN function. Perhaps it's something else entirely.