Skip to content

[CHORE] Fix heuristic bug on eager matching#49

Merged
vyasworks merged 1 commit intomainfrom
eager_fix1
Feb 8, 2023
Merged

[CHORE] Fix heuristic bug on eager matching#49
vyasworks merged 1 commit intomainfrom
eager_fix1

Conversation

@vyasworks
Copy link
Contributor

Objective:

Fix bug that does not mark match=true for certain deposits when they were matched.

Issue - reuse of deposits for matching after they were already used for matches!

differenceInSeconds(payment.timestamp, deposit.timestamp) < 240
) {

// mutate the original array to use in next heurisitc match
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This similar problem will exist in cash totals as well, needs fix

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we implement the layered approach here, but invert it. So first we check 1:1 matching where difference in seconds is very small, then if no matches, continue to broaden the window until there are none yet left to be matched for the date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

start with 10 seconds and broaden until 240 (or whatever upper limit is) seconds? Then yes.

if more, then no.

Copy link
Contributor

@chelsea-EYDS chelsea-EYDS Feb 8, 2023

Choose a reason for hiding this comment

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

Okay, so there is no case in which a transaction occurs and the deposit could be delayed greater than 240 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there could be but having a greater time span make more possibility for bad matches than good ones? this could be wrong but how I understand it now. we can verify with BAs.

@vyasworks vyasworks merged commit 2d17e57 into main Feb 8, 2023
@vyasworks vyasworks deleted the eager_fix1 branch February 8, 2023 17:24
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