Skip to content

Refactoring test by Kate Butler #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactoring test by Kate Butler #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 52 additions & 3 deletions RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace RefactorThis.Domain.Tests
public class InvoicePaymentProcessorTests
{
[Test]
public void ProcessPayment_Should_ThrowException_When_NoInoiceFoundForPaymentReference( )
public void ProcessPayment_Should_ThrowException_When_NoInvoiceFoundForPaymentReference( )
{
var repo = new InvoiceRepository( );

Expand Down Expand Up @@ -142,6 +142,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis
var repo = new InvoiceRepository( );
var invoice = new Invoice( repo )
{
Type = InvoiceType.Standard,
Amount = 10,
AmountPaid = 5,
Payments = new List<Payment>
Expand All @@ -164,6 +165,44 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis
var result = paymentProcessor.ProcessPayment( payment );

Assert.AreEqual( "final partial payment received, invoice is now fully paid", result );
Assert.AreEqual( 10, invoice.AmountPaid );
Assert.AreEqual( 0m, invoice.TaxAmount );
Assert.AreEqual( 2, invoice.Payments.Count );
}

[Test]
public void ProcessPayment_Should_ApplyTaxToCommercialInvoice_When_PartialPaymentExistsAndAmountPaidEqualsAmountDue( )
{
var repo = new InvoiceRepository( );
var invoice = new Invoice( repo )
{
Type = InvoiceType.Commercial,
Amount = 10,
AmountPaid = 5,
TaxAmount = 0.7m,
Payments = new List<Payment>
{
new Payment
{
Amount = 5
}
}
};
repo.Add( invoice );

var paymentProcessor = new InvoiceService( repo );

var payment = new Payment( )
{
Amount = 5
};

var result = paymentProcessor.ProcessPayment( payment );

Assert.AreEqual( "final partial payment received, invoice is now fully paid", result );
Assert.AreEqual( 10, invoice.AmountPaid );
Assert.AreEqual( 1.4m, invoice.TaxAmount );
Assert.AreEqual( 2, invoice.Payments.Count );
}

[Test]
Expand All @@ -174,7 +213,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx
{
Amount = 10,
AmountPaid = 0,
Payments = new List<Payment>( ) { new Payment( ) { Amount = 10 } }
Payments = new List<Payment>( )
};
repo.Add( invoice );

Expand All @@ -187,7 +226,10 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx

var result = paymentProcessor.ProcessPayment( payment );

Assert.AreEqual( "invoice was already fully paid", result );
Assert.AreEqual( "invoice is now fully paid", result );
Assert.AreEqual( 10, invoice.AmountPaid );
Assert.AreEqual( 1.4m, invoice.TaxAmount );
Assert.AreEqual( 1, invoice.Payments.Count );
}

[Test]
Expand All @@ -196,6 +238,7 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment
var repo = new InvoiceRepository( );
var invoice = new Invoice( repo )
{
Type = InvoiceType.Commercial,
Amount = 10,
AmountPaid = 5,
Payments = new List<Payment>
Expand All @@ -218,6 +261,9 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment
var result = paymentProcessor.ProcessPayment( payment );

Assert.AreEqual( "another partial payment received, still not fully paid", result );
Assert.AreEqual( 6, invoice.AmountPaid );
Assert.AreEqual( 0.14m, invoice.TaxAmount );
Assert.AreEqual( 2, invoice.Payments.Count );
}

[Test]
Expand All @@ -242,6 +288,9 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPayme
var result = paymentProcessor.ProcessPayment( payment );

Assert.AreEqual( "invoice is now partially paid", result );
Assert.AreEqual( 1, invoice.AmountPaid );
Assert.AreEqual( 0.14m, invoice.TaxAmount );
Assert.AreEqual( 1, invoice.Payments.Count );
}
}
}
187 changes: 69 additions & 118 deletions RefactorThis.Domain/InvoiceService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,133 +17,84 @@ public string ProcessPayment( Payment payment )
{
var inv = _invoiceRepository.GetInvoice( payment.Reference );

var responseMessage = string.Empty;

if ( inv == null )
{
throw new InvalidOperationException( "There is no invoice matching this payment" );
}
else

if ( inv.Amount == 0 )
{
if ( inv.Amount == 0 )
{
if ( inv.Payments == null || !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.HasPayments( ) )
{
if ( inv.Payments != null && inv.Payments.Any( ) )
{
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( );
}
}
}
}
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( );
}
}
}
throw new InvalidOperationException(
"The invoice is in an invalid state, it has an amount of 0 and it has payments.");
}

return "no payment needed";
}

return ( inv.HasPayments( ) )
? ProcessPartialPayment( inv, payment )
: ProcessFullPayment( inv, payment );
}

private static string ProcessPartialPayment( Invoice inv, Payment payment )
{
if ( inv.Amount == inv.Payments.Sum( x => x.Amount ) )
{
return "invoice was already fully paid";
}

if ( payment.Amount > ( inv.Amount - inv.AmountPaid ) )
{
return "the payment is greater than the partial amount remaining";
}

inv.Save();

return responseMessage;
switch ( inv.Type )
{
case InvoiceType.Standard:
// TODO: Discuss with stakeholders
// I'm preserving existing behaviour here - but it seems wrong!
// I suspect that either this is supposed to be taxed - or that Standard
// invoices should not be taxed in the ProcessFullPayment() function either
inv.AddPayment( payment, false );
break;
case InvoiceType.Commercial:
inv.AddPayment( payment );
break;
default:
throw new ArgumentOutOfRangeException( );
}

inv.Save( );

return ( inv.AmountPaid == inv.Amount )
? "final partial payment received, invoice is now fully paid"
: "another partial payment received, still not fully paid";
}

private static string ProcessFullPayment( Invoice inv, Payment payment )
{
if ( payment.Amount > inv.Amount )
{
return "the payment is greater than the invoice amount";
}

switch ( inv.Type )
{
case InvoiceType.Standard:
case InvoiceType.Commercial:
inv.AddPayment( payment );
break;
default:
throw new ArgumentOutOfRangeException( );
}

inv.Save( );

return ( inv.Amount == payment.Amount )
? "invoice is now fully paid"
: "invoice is now partially paid";
}
}
}
15 changes: 15 additions & 0 deletions RefactorThis.Persistence/Invoice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Linq;

namespace RefactorThis.Persistence
{
Expand All @@ -21,6 +22,20 @@ public void Save( )
public List<Payment> Payments { get; set; }

public InvoiceType Type { get; set; }

public bool HasPayments( )
{
return Payments != null && Payments.Any( );
}
public void AddPayment( Payment payment, bool isTaxable = true )
{
AmountPaid += payment.Amount;
if (isTaxable)
{
TaxAmount += payment.Amount * 0.14m;
}
Payments.Add( payment );
}
}

public enum InvoiceType
Expand Down