# API Design Observations
Notes captured from today’s pass through `gametools`, organized around the Rust API Guidelines checklist.
## Cards module
- `Deck<T>` lives behind `impl<T: CardFaces + Clone>` even for methods that don’t clone (`shuffle`, `deal`, etc.), violating the guideline to keep generic bounds minimal (C-GENERIC). Only constructors that consume/clones faces/cards should require `Clone`.
- `Deck::new_with_cards` clones the entire slice passed in before storing it, preventing the use of non-`Clone` faces and surprising callers who thought they were handing over ownership. Consuming an owned iterator (`impl IntoIterator<Item = Card<T>>`) would avoid the extra allocation and follow the “functions minimize assumptions about inputs” principle.
- `Deck::new_with_faces` only accepts `&[T]` and always clones each face. Add an owned version (e.g., `impl IntoIterator<Item = T>`) so callers who already own the faces don’t pay for redundant clones.
- `Deck`, `Hand`, and `Pile` expose their `cards` vectors as `pub`. That makes it impossible to uphold invariants (e.g., `DeckId` tagging, top-of-stack semantics) and conflicts with C-ENCAPSULATION. Hide the fields and offer read-only slices or iterators instead.
## Spinners module
- The main `impl` block forces `T: Clone + PartialEq`, even though `spin`, `iter`, etc., never compare values. Split impls so non-comparison helpers only need `Clone`, satisfying C-GENERIC.
- Methods like `cover`, `uncover`, `remove_wedges`, and `replace_value` take their match value by value rather than by reference. Accepting `&T` (or `impl Borrow<T>`) aligns with the “borrow for inputs, own for outputs” rule of thumb and avoids needless cloning by callers.
- `Spinner::wedges(&self)` returns a cloned `Vec`, which is expensive for read-only inspection. Expose `&[Wedge<T>]` (plus the existing iterator) per the guideline to provide slice access rather than forcing allocation.
## Dominos module
- `DominoHand::play_line` takes `&Vec<usize>`, which the guidelines call out as undesirable in public APIs. Switch to `&[usize]` (or a generic iterator) so callers can pass any contiguous collection without converting.
## Next steps
1. Refactor `Deck` and `Spinner` impl blocks to narrow trait bounds and update call sites/tests accordingly.
2. Change the deck constructors to consume owned collections (and add optional slice helpers) so cloning is only done when necessary.
3. Make collection fields private and provide accessor methods to keep invariants enforceable.
4. Update spinner/domino parameter types to borrow rather than own where appropriate.