Skip to content

[210_5] 实现 either-flat-map#696

Merged
MoonL79 merged 2 commits intomainfrom
cms/210_5/either_flat_map
Apr 13, 2026
Merged

[210_5] 实现 either-flat-map#696
MoonL79 merged 2 commits intomainfrom
cms/210_5/either_flat_map

Conversation

@MoonL79
Copy link
Copy Markdown
Collaborator

@MoonL79 MoonL79 commented Apr 13, 2026

No description provided.

@MoonL79 MoonL79 self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Contributor

@da-liii da-liii left a comment

Choose a reason for hiding this comment

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

LGTM

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 13, 2026

Greptile Summary

This PR adds either-flat-map (monadic bind) to (liii either), allowing Right values to be chained through functions that themselves return an Either — short-circuiting on Left.

  • P1: either-flat-map does not validate that f returns an Either. If a plain-value function is passed, the result is silently a non-Either, breaking the type invariant for all downstream either-* calls. A check-either on f's return value is needed.

Confidence Score: 4/5

Safe to merge after adding return-value validation in either-flat-map.

There is one P1 correctness issue: the return value of f is not validated as an Either, so a misuse silently returns a non-Either and defers the error to an unrelated call site. All other findings are P2 (missing test coverage for short-circuit chaining). Addressing the P1 validation gap before merging is recommended.

goldfish/liii/either.scm — the either-flat-map implementation needs a check-either guard on the return value of f.

Important Files Changed

Filename Overview
goldfish/liii/either.scm Adds either-flat-map export and implementation; return value of f is not validated as an Either, silently breaking the type contract when f returns a plain value.
tests/liii/either/either-flat-map-test.scm New test file covers basic Left pass-through, Right transform, Right-to-Left, and error handling; missing the short-circuit chaining case (Right → Left → short-circuit).
tests/liii/either-test.scm Adds index comment for the new either-flat-map function; no issues.
devel/210_5.md Adds a date entry for the new implementation; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["either-flat-map f either"] --> B{"check-either either"}
    B -- "not Either" --> C["type-error"]
    B -- "OK" --> D{"procedure? f"}
    D -- "not procedure" --> E["type-error"]
    D -- "OK" --> F{"either-right? either"}
    F -- "Left" --> G["return either unchanged"]
    F -- "Right" --> H["result = f (to-right either)"]
    H --> I{"⚠️ check-either result\n(currently missing)"}
    I -- "not Either" --> J["type-error (proposed)"]
    I -- "Either" --> K["return result"]
Loading

Reviews (1): Last reviewed commit: "[210_5] 实现 either-flat-map" | Re-trigger Greptile

Comment on lines +131 to +134
(if (either-right? either)
(f (to-right either))
either
) ;if
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Return value of f not validated

either-flat-map is a monadic bind — f is expected to return an Either. If a caller passes a plain-value function (e.g. (lambda (x) (* x 2))), the result is silently a non-Either, breaking the type invariant for every downstream either-* call. The error then surfaces with a confusing message in the downstream function rather than here.

Contrast with either-map, which wraps f's result in from-right unconditionally. For flat-map the wrapping is intentionally skipped, so an explicit runtime check is needed instead:

(if (either-right? either)
    (let ((result (f (to-right either))))
      (check-either result "either-flat-map: return value of f must be an Either")
      result)
    either
) ;if

Comment on lines +49 to +54
(let* ((val1 (from-right 10))
(val2 (either-flat-map (lambda (x) (from-right (+ x 5))) val1))
(val3 (either-flat-map (lambda (x) (from-right (* x 2))) val2)))
(check-true (either-right? val3))
(check (to-right val3) => 30)
) ;let*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Missing short-circuit chaining test

The existing chaining test only covers Right → Right → Right. The key monadic property is that once a step returns Left, all subsequent flat-map calls must pass it through untouched. Adding a test like:

;; Right -> Left -> (short-circuit) -> Left
(let* ((val1 (from-right 7))
       (val2 (either-flat-map (lambda (x) (from-left "err")) val1))
       (val3 (either-flat-map (lambda (x) (from-right (* x 2))) val2)))
  (check-true (either-left? val3))
  (check (to-left val3) => "err")
) ;let*

would explicitly verify the short-circuit behaviour that is the defining feature of monadic chaining.

@MoonL79 MoonL79 merged commit cdb48fe into main Apr 13, 2026
4 checks passed
@MoonL79 MoonL79 deleted the cms/210_5/either_flat_map branch April 13, 2026 02:37
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.

2 participants