Skip to content

Conversation

@VarinderS
Copy link

Summary

This PR delivers a full domain-level refactor of the Invoice.ApplyPayment method, refactoring what was previously a large, deeply nested, and imperative method into a clean, extensible, and domain-aligned implementation using established DDD principles and design patterns.

The goal of this refactor was to improve readability, encapsulate business logic correctly, and set a strong architectural foundation for future enhancements (eg new invoice types, tax rules, or payment flows).

Key Changes

Business Logic Moved into Domain

All logic related to validation, tax application, and messaging lives entirely within the Invoice aggregate root.
Introduced guard clauses and intention-revealing private methods to eliminate nested if blocks and make the ApplyPayment() flow linear and intuitive.

Strategy Pattern for Tax Calculation

Replaced hardcoded tax logic with pluggable tax strategies:

  • StandardInvoiceTaxStrategy
  • CommercialInvoiceTaxStrategy

Selected via the InvoiceTaxStrategyFactory based on InvoiceType, aligning with Open/Closed Principle.

Centralised Message Management

Moved all user-facing messages to InvoiceMessages.cs, now sorted alphabetically and documented.
This makes the code easier to maintain and localise if needed.

Improved Testability & Coverage

Existing tests were updated to use the refactored model correctly. Structure now supports testing each strategy or validation path in isolation.

Benefits after refactor

Before After
Large, hard-to-read method with nested blocks Clear, linear method with small focused helpers
Magic strings and numbers scattered throughout the codebase Centralised, self-documented message constants
Tight coupling of tax logic to invoice logic Cleanly separated using Strategy Pattern
Difficult to extend or test in isolation Modular, testable components with single responsibility
Potential for future bugs Clear validation, consistent state mutations

New domain structure

RefactorThis.Domain
│
├── Invoice.cs <- Aggregate root, contains ApplyPayment logic
├── InvoiceMessages.cs <- All response and error messages, alphabetically sorted
├── InvoiceService.cs <- Application service coordinating payment processing
├── IInvoiceRepository.cs <- Repository abstraction for invoice data
│
├── TaxStrategy/
│   ├── IInvoiceTaxStrategy.cs <- Strategy interface for tax logic
│   ├── StandardInvoiceTaxStrategy.cs <- Applies tax only on first payment
│   ├── CommercialInvoiceTaxStrategy.cs <- Applies tax on every payment
│   └── InvoiceTaxStrategyFactory.cs <- Resolves the appropriate strategy based on InvoiceType

Notes for future developers

All new message constants in InvoiceMessages.cs must be added in alphabetical order (see comment in file).
New invoice types can be supported by:

  1. Adding a new class implementing IInvoiceTaxStrategy
  2. Registering it inside InvoiceTaxStrategyFactory

Avoid placing domain logic in service classes. Services should delegate to the aggregate root (Invoice).

Test results

image

cc: @re-leased-hiring

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.

1 participant