Flatten parallel if-let chains; codify the rule in CLAUDE.md#113
Merged
Flatten parallel if-let chains; codify the rule in CLAUDE.md#113
Conversation
Five spots had 3+ if-lets each doing parallel "if Some, do one thing"
work — typically a builder/insert/push chain that's just a struct
literal or filter_map under the syntactic noise:
- create_link (services/links/service.rs): 9 if-lets unwrap-then-builder
→ struct literal. Drops the entire builder impl on CreateLinkInput
since the methods become unreferenced.
- update_link (services/links/service.rs): 6 conditional Document inserts
→ string and serializable-struct branches collapsed via filter_map +
Document::extend, with a small `to_doc_value` helper for the bson
encoding.
- apply_subscription_update (services/auth/tenants/repo.rs): 4 infallible
inserts collapsed via filter_map; the 3 fallible bson encodings stay
separate so `?` lifts the error inline.
- JSON-LD entry_points (api/links/routes.rs): 3 if-lets pushing
EntryPoint json! objects → filter_map over a [(opt, platform)] array.
- Landing-page dests Vec (api/links/routes.rs): 5 if-lets pushing
("label", url) tuples → filter_map.
Adds a Style Guidelines bullet to CLAUDE.md so future code review
flags this shape: "Three or more if let statements in a row applying
parallel logic is a smell — there's almost always a flatter form."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…n create_link Walks back the builder deletion. The builder pattern is the right fit for a struct with many optional fields — the original design just took T in every setter, which forced every caller into an if-let chain to unwrap an Option<T> source. Setters now take Option<T> directly so create_link can fluent-chain straight from the request struct without unwrapping. Updates CLAUDE.md to add fluent builders to the list of valid fixes for the if-let-chain smell, with a design note that taking T (instead of Option<T>) reintroduces the smell at every call site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the most egregious "if-let chain doing parallel work" cases in the codebase and adds a Style Guidelines bullet to CLAUDE.md so the pattern gets flagged in review.
The rule (added to
CLAUDE.mdStyle Guidelines):Audit hits fixed:
create_linkupdate_linkupdate.insert(...)calls (3 nested with bson encoding)Document::extend+ smallto_doc_valuehelperapply_subscription_updateset_doc.insert(...)calls?) + 4 infallible collapsed into one filter_mapentry_pointsjson!({...})per platform[(opt, platform_tag)]destsVec("label", url)tuplesSide effect: since
create_linkwas the last caller of the chained-builder methods onCreateLinkInput, the entireimpl CreateLinkInputblock (10 builder methods +new) is now unreferenced and gets deleted. Per CLAUDE.md "fix root cause, don't#[allow(dead_code)]."Left alone deliberately:
validate_link_urls,validate_agent_context,validate_social_previeweach have if-let chains, but every branch calls a distinct validator with a distinct error label and uses?. The flatter form (array-of-tuples +try_for_each) reads worse there. The rule says "almost always" not "always."Test plan
cargo fmt -- --check,cargo clippy --all-targets -- -D warnings,cargo test(86 lib + 145 integration) all greenupdate_link's bson encoding (matching the pre-refactor behavior)Note on stacking with #112
This branches off
main, notbulk-links-create. The bulk PR (#112) also uses a struct-literalCreateLinkInput— neither PR re-introduces the builder methods, so they're orthogonal regardless of merge order.🤖 Generated with Claude Code