4
\$\begingroup\$

I work at a major Swedish insurance company. I'm in charge of a service that matches bank payments with invoices. I will not show the details of that match logic. But one outcome of the match is that amount on payment differs from amount on invoice. We have a IAmountDeviationStrategy that is tagged on each invoice. Here is the RefundStrategy

public class RefundStrategy : IAmountDeviationStrategy
{
    public AmountDeviationStrategy Strategy => AmountDeviationStrategy.Refund;

    public PaymentInvoiceMatchResult Execute(MatchJob job)
    {
        if (job.Diff < 0)
            return new PaymentInvoiceMatchResultBuilder(job)
                .PaymentCategory(PaymentCategory.WrongPayment)
                .RefundAll(RefundReason.UnderPaid);

        var invoiceAmount = Math.Abs(job.Invoice.Amount);
        var payingPayments = GetPaymentsComboClosestToInvoiceAmount(job.Payments, invoiceAmount);

        var refunds = job.Payments.Except(payingPayments).ToList();
        var result = new PaymentInvoiceMatchResultBuilder(job, refunds)
            .PaymentCategory(PaymentCategory.WrongPayment)
            .Refund(refunds);

        if (payingPayments.Sum(p => p.Amount) != invoiceAmount)
            result.Refund<RefundSplitPaymentCommand>(payingPayments.First(p => p.Amount >= job.Diff), split => split.Amount = payingPayments.Sum(p => p.Amount) - invoiceAmount);

        return result
            .QueueCommand(new MapInvoicePaymentsCommand { InvoiceId = job.Invoice.ExternalId, Payments = payingPayments.Select(p => p.ExternalId) });
    }

    private IReadOnlyCollection<IPaymentMatchInfo> GetPaymentsComboClosestToInvoiceAmount(IReadOnlyCollection<IPaymentMatchInfo> payments, decimal invoiceAmount)
    {
        var combos = Enumerable.Range(1, payments.Count)
            .SelectMany(i => new Combinations<IPaymentMatchInfo>(payments, new ReferenceCompare<IPaymentMatchInfo>(), i))
            .ToList();

        var payingPayments = combos
            .Where(c => c.Sum(p => p.Amount) >= invoiceAmount)
            .OrderBy(combo => Math.Abs(combo.Sum(p => p.Amount) - invoiceAmount))
            .First();

        return payingPayments;
    }

If the payment underpays the invoice we simlply refund the payment. If its overpaid the fun starts. There can be multiple payments. Most common is the customer have paid once and overpaid. Second most common is he have double paid, ie two payments which sums to 2x the invoice. Third and uncommon is multiple payments that overpays randomly (not double the amount).

Anyway, I bruteforce this problem. I find all combos of payments using a little Combination class https://pastebin.com/BDvRmmgD (Not written by me)

Once I have all combos I take the one combo closest to the invoice amount. I refund all others. I then check if the paying payments sum up to the amount (which they do in double paid scenario). If they dont sum up I split one of the payments and refund the overpaid part.

I then rerun the matching command MapInvoicePaymentsCommand. This time the invoice and payment will hit the Paid outcome path instead.

I think the brute force part is pretty elegant. There will never be more payments than a few so it will not be a performance problem.

\$\endgroup\$
11
  • \$\begingroup\$ Consider a title that describes the program rather than asking a question. Thanks. \$\endgroup\$
    – ggorlen
    Sep 2, 2021 at 5:19
  • \$\begingroup\$ @ggorlen Ok. I dont agree the new title is better. But there you go \$\endgroup\$
    – Anders
    Sep 2, 2021 at 8:21
  • \$\begingroup\$ Thanks for your flexibility! Looks great to me. If you browse codereview.stackexchange.com for a few pages, it might convince you that it fits the site better. There's a push to remove the word "this" from titles, because it doesn't really tell you what you're clicking on. \$\endgroup\$
    – ggorlen
    Sep 2, 2021 at 14:36
  • 1
    \$\begingroup\$ Ah, but this is part of what gets you better feedback on the actual code/domain problem. Posts that are less on topic and have an unclear title => fewer upvotes, less attention, worse (or no) reviews... if it's off-topic and unclear enough, downvotes and closure ensue. If you have a crisper title and topic, you get more upvotes, more attention and better reviews. Also, Stack Exchange sites are about the community benefit foremost -- it's about curating a resource for future visitors and building a body of knowledge that helps everyone learn. \$\endgroup\$
    – ggorlen
    Sep 2, 2021 at 14:57
  • 1
    \$\begingroup\$ Based on the old title, what's your problem domain? Your "use case" might be brute-forcing a password, brute forcing a chess puzzle, brute forcing a Project Euler or Leetcode problem... who knows? In fact, it's misleading, because I'm more interested in those algorithmic topics than in enterprise C# business logic, while others who might be interested in a business app would have more reason to visit post-update. It goes without saying that you're asking for review of your approach ("is this an elegant approach?"). That applies to every post on CR and provides no differentiating value. \$\endgroup\$
    – ggorlen
    Sep 2, 2021 at 15:15

1 Answer 1

1
\$\begingroup\$

Maybe it's elegant maybe not, but IMHO it's not predicted behavior.

Let's say my invoice is for 100$. For some circumstances I paid in the following order:

  • $60
  • $100
  • $70

What is predicted behavior here?

The software is going to fill the invoice in chronological order (as my payments were). So my first $60 payment should be confirmed for sure, next from my $100 payment $40 should be confirmed (as it was remaining to fill invoice) and other $60 should be refunded as well as my 3rd payment because by the time I did the 3'rd payment invoice was already filled.

If it's not an explicit business requirement I suggest, simply ordering payments in chronological order and 1 by 1 filling the invoice. When it gets filled, refund everything remained.

Just imagine how many "Why this was refunded?" questions will arise and how much time will spend accountant on brute-forcing in his/her head while looking on the refunds spreadsheet.

It's both predictable for customers and for your future college who is going to work in this code.

Again if there is no specific business reason for that.

\$\endgroup\$
4
  • \$\begingroup\$ In your example it will keep the 100 USD and refund the two others. I think its the most correct approuch. Otherwise it would take 60 from the first 40 from the second and refund 70 + 60. Thats more strange to me. \$\endgroup\$
    – Anders
    Sep 24, 2021 at 10:54
  • 1
    \$\begingroup\$ "Fill invoice as payments arrive." vs "Decide what amounts are perfectly matching for the invoice amount." Why? Imho you should keep simple as far as there is no other concern. To me, the first approach seems simplest. \$\endgroup\$ Sep 24, 2021 at 19:02
  • \$\begingroup\$ It's by design and per requirements \$\endgroup\$
    – Anders
    Sep 24, 2021 at 22:22
  • \$\begingroup\$ I know more know about why it works as is. Its for accounting reasons both for our own accounting and for the customers. You want to split wrong payments only if really needed. \$\endgroup\$
    – Anders
    Jan 12 at 8:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.