Remix.run Logo
sublinear 3 days ago

> The Uncle Bob approach of extractNameMatchingTransfersWithinSettlementWindow() doesn't actually help - now I need to know what settlement windows are anyway, and I've lost the context of why 3 days.

When the name is long, it should be more than one function. Finding the transactions within a settlement window seems distinct from matching them up.

The comment about 3 days isn't necessary when "numDays" is a variable in the SettlementWindow class (or other relevant scope). I'd be a lot more comfortable reading code that makes this concept a proper abstraction rather than just seeing a comment about a hard-coded 3 somewhere. Magic values are bad and comments don't demystify anything if they get out of sync with the code.