fix(Route): Enforce no empty swaps on Router::new()#258
Conversation
Took 18 minutes Took 11 seconds
dianacarvalho1
force-pushed
the
maintenance/dc/ENG-6023-enforce-no-empty-swaps-in-route
branch
from
501eae5 to
33bf08b
Compare
There was a problem hiding this comment.
Thank you @dianacarvalho1! 🤩
I like the change and it looks good to me, only left one question!
| // Constructs a `Route` directly, bypassing `Route::new`'s non-empty check, so | ||
| // accessor behaviour on empty routes (reachable via deserialization) stays testable. | ||
| fn make_route(swaps: Vec<(u8, u8)>) -> Route { | ||
| let swaps: Vec<Swap> = swaps | ||
| .into_iter() | ||
| .map(|(a, b)| make_swap(a, b, 1000, 990)) | ||
| .collect(); | ||
| Route::new(swaps, HashMap::new()) | ||
| Route { swaps, tokens: HashMap::new() } |
There was a problem hiding this comment.
If we enforce to not have empty swaps in the Route, why do we need to keep it testable?
There was a problem hiding this comment.
when we create a Route through deserialization it is still possible to have empty swaps
There was a problem hiding this comment.
but shouldn't we also have a check the result after the deserialization and make sure it is not empty?
There was a problem hiding this comment.
actually.. I think deserialisation never truly happens inside of Fynd. Route is only a response type. Enforcing at new closes the only realistic source (algorithm code) without touching the public API
2 participants
No description provided.