diff --git a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs index 3a607fd..d94bf6d 100644 --- a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs +++ b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs @@ -43,7 +43,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_NoPaymentNeeded( ) Payments = null }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -71,7 +71,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_InvoiceAlreadyFullyP } } }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -98,7 +98,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_PartialPaymentExists } } }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -122,7 +122,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_NoPartialPaymentExis AmountPaid = 0, Payments = new List( ) }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -152,7 +152,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis } } }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -176,7 +176,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx AmountPaid = 0, Payments = new List( ) { new Payment( ) { Amount = 10 } } }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -206,7 +206,7 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment } } }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); @@ -230,7 +230,7 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPayme AmountPaid = 0, Payments = new List( ) }; - repo.Add( invoice ); + repo.UpsertInvoice( invoice ); var paymentProcessor = new InvoiceService( repo ); diff --git a/RefactorThis.Domain/InvoiceService.cs b/RefactorThis.Domain/InvoiceService.cs index fbd674c..23a5d43 100644 --- a/RefactorThis.Domain/InvoiceService.cs +++ b/RefactorThis.Domain/InvoiceService.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Collections.Generic; using RefactorThis.Persistence; namespace RefactorThis.Domain @@ -8,6 +9,8 @@ public class InvoiceService { private readonly InvoiceRepository _invoiceRepository; + private const decimal TAX_RATE = 0.14m; + public InvoiceService( InvoiceRepository invoiceRepository ) { _invoiceRepository = invoiceRepository; @@ -17,126 +20,68 @@ public string ProcessPayment( Payment payment ) { var inv = _invoiceRepository.GetInvoice( payment.Reference ); - var responseMessage = string.Empty; - - if ( inv == null ) + // Fail fast if invoice not found + if (inv == null) { throw new InvalidOperationException( "There is no invoice matching this payment" ); } + + // Ensure payments collection is present so we can safely Add to it later. + inv.Payments = inv.Payments ?? new List(); + + string responseMessage; + + if ( inv.Amount == 0 ) + { + if ( !inv.Payments.Any() ) + { + responseMessage = "no payment needed"; + } + else + { + throw new InvalidOperationException( "The invoice is in an invalid state, it has an amount of 0 and it has payments." ); + } + } else { - if ( inv.Amount == 0 ) + if ( inv.Payments.Any() ) { - if ( inv.Payments == null || !inv.Payments.Any( ) ) + var paymentsSum = inv.Payments.Sum( x => x.Amount ); + if ( paymentsSum != 0 && inv.Amount == paymentsSum ) + { + responseMessage = "invoice was already fully paid"; + } + else if ( paymentsSum != 0 && payment.Amount > ( inv.Amount - inv.AmountPaid ) ) { - responseMessage = "no payment needed"; + responseMessage = "the payment is greater than the partial amount remaining"; } else { - throw new InvalidOperationException( "The invoice is in an invalid state, it has an amount of 0 and it has payments." ); + // There are previous payments and this payment is valid to apply + bool isFinalPayment = ( inv.Amount - inv.AmountPaid ) == payment.Amount; + ApplyPayment(inv, payment, replaceAmountPaid: false, firstPayment: false); + responseMessage = isFinalPayment + ? "final partial payment received, invoice is now fully paid" + : "another partial payment received, still not fully paid"; } } else { - if ( inv.Payments != null && inv.Payments.Any( ) ) + if ( payment.Amount > inv.Amount ) + { + responseMessage = "the payment is greater than the invoice amount"; + } + else if ( inv.Amount == payment.Amount ) { - if ( inv.Payments.Sum( x => x.Amount ) != 0 && inv.Amount == inv.Payments.Sum( x => x.Amount ) ) - { - responseMessage = "invoice was already fully paid"; - } - else if ( inv.Payments.Sum( x => x.Amount ) != 0 && payment.Amount > ( inv.Amount - inv.AmountPaid ) ) - { - responseMessage = "the payment is greater than the partial amount remaining"; - } - else - { - if ( ( inv.Amount - inv.AmountPaid ) == payment.Amount ) - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid += payment.Amount; - inv.Payments.Add( payment ); - responseMessage = "final partial payment received, invoice is now fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid += payment.Amount; - inv.TaxAmount += payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "final partial payment received, invoice is now fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - - } - else - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid += payment.Amount; - inv.Payments.Add( payment ); - responseMessage = "another partial payment received, still not fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid += payment.Amount; - inv.TaxAmount += payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "another partial payment received, still not fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } - } + // First payment that exactly equals the invoice amount + ApplyPayment(inv, payment, replaceAmountPaid: true, firstPayment: true); + responseMessage = "invoice is now fully paid"; } else { - if ( payment.Amount > inv.Amount ) - { - responseMessage = "the payment is greater than the invoice amount"; - } - else if ( inv.Amount == payment.Amount ) - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } - else - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now partially paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now partially paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } + // First payment and it's partial + ApplyPayment(inv, payment, replaceAmountPaid: true, firstPayment: true); + responseMessage = "invoice is now partially paid"; } } } @@ -145,5 +90,33 @@ public string ProcessPayment( Payment payment ) return responseMessage; } + + private void ApplyPayment(Invoice inv, Payment payment, bool replaceAmountPaid, bool firstPayment) + { + if (replaceAmountPaid) + { + inv.AmountPaid = payment.Amount; + } + else + { + inv.AmountPaid += payment.Amount; + } + + // Tax behavior mirrors original code: on the very first payment set TaxAmount = amount * rate; + // on subsequent payments only Commercial invoices accumulate additional tax. + if (firstPayment) + { + inv.TaxAmount = payment.Amount * TAX_RATE; + } + else + { + if (inv.Type == InvoiceType.Commercial) + { + inv.TaxAmount += payment.Amount * TAX_RATE; + } + } + + inv.Payments.Add(payment); + } } } \ No newline at end of file diff --git a/RefactorThis.Persistence/Invoice.cs b/RefactorThis.Persistence/Invoice.cs index bd4370d..b3aee0f 100644 --- a/RefactorThis.Persistence/Invoice.cs +++ b/RefactorThis.Persistence/Invoice.cs @@ -8,19 +8,23 @@ public class Invoice public Invoice( InvoiceRepository repository ) { _repository = repository; + // Ensure Payments is initialized to safely add Payments + Payments = new List(); + // Default to Standard to provide a sensible default + Type = InvoiceType.Standard; } public void Save( ) { - _repository.SaveInvoice( this ); + _repository.UpsertInvoice( this ); } public decimal Amount { get; set; } public decimal AmountPaid { get; set; } public decimal TaxAmount { get; set; } public List Payments { get; set; } - public InvoiceType Type { get; set; } + public string Reference { get; set; } } public enum InvoiceType diff --git a/RefactorThis.Persistence/InvoiceRepository.cs b/RefactorThis.Persistence/InvoiceRepository.cs index 38548c7..ca47849 100644 --- a/RefactorThis.Persistence/InvoiceRepository.cs +++ b/RefactorThis.Persistence/InvoiceRepository.cs @@ -1,21 +1,48 @@ +using System; +using System.Collections.Generic; +using System.Linq; + namespace RefactorThis.Persistence { public class InvoiceRepository { - private Invoice _invoice; + private readonly List _invoices = new List(); - public Invoice GetInvoice( string reference ) + public Invoice GetInvoice(string reference) { - return _invoice; - } + if (string.IsNullOrWhiteSpace(reference)) + { + return _invoices.FirstOrDefault(); + } - public void SaveInvoice( Invoice invoice ) - { - //saves the invoice to the database + return _invoices.FirstOrDefault(i => + string.Equals(i.Reference, reference, StringComparison.OrdinalIgnoreCase)); } - public void Add( Invoice invoice ) + public void UpsertInvoice(Invoice invoice) { - _invoice = invoice; + if (invoice == null) throw new ArgumentNullException(nameof(invoice)); + + Invoice existing = null; + if (!string.IsNullOrWhiteSpace(invoice.Reference)) + { + existing = _invoices.FirstOrDefault(i => + string.Equals(i.Reference, invoice.Reference, StringComparison.OrdinalIgnoreCase)); + } + + if (existing == null) + { + existing = _invoices.FirstOrDefault(i => ReferenceEquals(i, invoice)); + } + + if (existing != null) + { + var index = _invoices.IndexOf(existing); + _invoices[index] = invoice; + } + else + { + _invoices.Add(invoice); + } } } } \ No newline at end of file