RefactorTest

A realistic .NET 8 refactoring exercise built around a legacy payment and reporting platform.

---

Project Purpose

RefactorTest is a hands-on technical assessment designed for Senior C#/.NET engineers. It simulates the kind of codebase you might inherit at a high-growth product company: functionally correct, covered by tests, but accumulated years of organic change.

What this project is

• A timed refactoring exercise (typically 60–90 minutes) focused on improving maintainability without changing behavior.
• A working .NET 8 solution with three projects: API, Core library, and unit tests.
• A deliberately imperfect codebase containing common real-world smells found in production systems.

What candidates are expected to do

1. Read the business requirements and run the existing test suite.
2. Explore the code and identify areas that hurt readability, testability, or extensibility.
3. Refactor incrementally while keeping all tests green.
4. Be prepared to explain why each change improves the design and what trade-offs were considered.

What this project is not

• It is not a greenfield feature exercise.
• It is not broken on purpose — the code compiles, runs, and passes tests.
• It is not a puzzle with a single “correct” answer. Reasonable engineers may prioritize different improvements within the time box.

Intended audience

• Hiring managers and interviewers looking for a standardized refactoring loop.
• Engineers practicing systematic code improvement before interviews.
• Teams onboarding developers onto brownfield .NET codebases.

---

Solution Overview

Project	Description
RefactorTest.Core	Domain models, repositories, services, exporters
RefactorTest.Api	ASP.NET Core Web API exposing REST endpoints
RefactorTest.Tests	xUnit tests verifying current behavior

The system uses an in-memory data store with seed data. No external database is required.

---

Business Requirements

Customers

• Customers can register with a name, email, and country code.
• Customers have an active/inactive status. Inactive customers cannot place orders or make payments.
• The system tracks lifetime spend per customer.
• Customers with lifetime spend above $1,000 receive a 5% VIP discount on new payments.
• Customer profiles show order history, totals, and an activity level (low, medium, or high based on order count).

Merchants

• Merchants register with a name, category, and commission rate (0–50%).
• Only active merchants can receive new orders.
• Merchant statistics include total orders, completed orders, gross revenue, and total commission for a given period.

Orders

• Orders are created against an active customer and merchant with a positive amount and description.
• New orders start in Pending status.
• Customers may have at most 5 pending orders at a time.
• Order statuses: Pending, Confirmed, Shipped, Completed, Cancelled.
• Completed and cancelled orders cannot be changed to other statuses.
• Order summaries include customer name, merchant name, processing fee, commission, and customer tier.

Payments

• Payments can be processed for non-cancelled orders using card or bank_transfer.
• Card payments incur a fee of 2.9% + $0.30. Bank transfers incur a 1% fee.
• Processing a pending order confirms it; successful payment captures funds and marks the order completed.
• VIP customers receive a 5% discount applied to the payment amount.
• Only captured payments can be refunded. Refunding cancels the associated order.
• Duplicate capture attempts for the same order return the existing captured payment.

Reports

• Daily Sales — completed orders for a single day, grouped by merchant category.
• Merchant Summary — orders and commission for a merchant over a date range.
• Customer Activity — orders and payment status for a customer over a date range.
• Payment Reconciliation — payments grouped by status with fee totals.
• Reports can be exported as CSV, JSON, or XML.

Exporters

• CSV, JSON, and XML exporters support customer, merchant, order, payment, and report data.
• XML export supports customers, merchants, and reports only.

---

Setup

Prerequisites

• .NET 8 SDK

Build

dotnet build

Run the API

dotnet run --project src/RefactorTest.Api

The API starts at https://localhost:7xxx (see console output). Swagger UI is available in Development at /swagger.

---

How to Run Tests

dotnet test

Run tests for a specific project:

dotnet test tests/RefactorTest.Tests

Run with verbose output:

dotnet test --verbosity normal

All tests assert current behavior. Any refactoring must keep them passing.

---

Answer Key

Note for interviewers: This section documents intentional design flaws. Share it with candidates after the exercise, not before.

The table below maps known issues to the principle they violate, the affected location, and a recommended direction. Multiple valid refactorings exist; the suggestions describe a clean, pragmatic target state.

SOLID Violations

Single Responsibility Principle (SRP)

Location	Issue	Recommended action
PaymentProcessor.ProcessPaymentAsync	One method validates entities, updates order status, calculates fees/discounts, persists payment, updates customer lifetime spend, and logs.	Extract focused collaborators: PaymentValidator, FeeCalculator (already exists but unused), OrderStatusUpdater, and keep orchestration thin.
OrderService.CreateOrderAsync	Mixes input validation, authorization checks, business rules, persistence, and logging.	Split validation (OrderCreationValidator) from orchestration.
ReportService	Acts as a report facade, export coordinator, revenue aggregator, and customer counter.	Split into ReportQueryService, ReportExportService, and analytics helpers.
ReportGenerator	Builds report content and assigns IDs and persists to InMemoryDataStore.	Separate report composition from persistence; inject IReportRepository.
IDataExporter	Combines serialization, binary export, file I/O, and capability metadata in one abstraction.	Split into IReportSerializer, IFileWriter, and optional capability interfaces.

Open/Closed Principle (OCP)

Location	Issue	Recommended action
ReportService.GenerateReportAsync	Long if/else chain on ReportType; new report types require editing this method.	Replace with a strategy/registry: IReportBuilder per ReportType, resolved via DI.
ExporterFactory	String and enum switches to instantiate exporters.	Register exporters in DI (IEnumerable<IDataExporter> or keyed services) and resolve by GetFormatName().
PaymentProcessor (fee logic)	if/else on paymentMethod with duplicated card/default branch.	Strategy per payment method: IPaymentFeeStrategy with CardFeeStrategy, BankTransferFeeStrategy.
ReportService.ExportReport	Redundant if/else branches that all call the same method.	Remove branching; delegate directly to exporter after factory resolution.

Liskov Substitution Principle (LSP)

Location	Issue	Recommended action
XmlDataExporter	ExportOrders, ExportPayments, and ExportReportAsBinary throw NotSupportedException. Callers using IDataExporter cannot safely substitute XML.	Use smaller interfaces (ICustomerExporter, IReportExporter) or a capability model (CanExport(entityType)). Do not force unsupported operations behind a shared contract.

Interface Segregation Principle (ISP)

Location	Issue	Recommended action
IDataExporter	8 methods; XML only supports a subset; not all consumers need file I/O or binary export.	Segregate by entity type and concern. Consumers depend only on what they use.
IReportService	11 methods spanning generation, export, and analytics.	Split into IReportGenerator, IReportReader, IReportExporter, IRevenueAnalytics.

Dependency Inversion Principle (DIP)

Location	Issue	Recommended action
PaymentProcessor	Concrete class injected into PaymentService; no abstraction boundary.	Introduce IPaymentProcessor and register via DI.
ReportService.ExportReport	Instantiates ExporterFactory with new instead of injected dependency.	Inject IExporterFactory or IExporterResolver.
ReportService, ReportGenerator	Depend on concrete InMemoryDataStore instead of repository abstractions.	Introduce IReportRepository and hide storage details.
ExporterFactory	new CsvDataExporter() etc. inside factory.	Register exporters in DI container; factory becomes a resolver.

---

DRY Violations (Don't Repeat Yourself)

Location	Issue	Recommended action
PaymentProcessor vs FeeCalculator	Fee calculation duplicated inline; FeeCalculator exists but is never used.	Inject and use FeeCalculator everywhere fees are computed.
OrderService.GetOrderSummaryAsync	Processing fee formula duplicated (amount * 0.029 + 0.30).	Centralize in FeeCalculator.CalculateProcessingFee.
ReportGenerator.BuildMerchantSummaryReport vs MerchantService.GetMerchantStatsAsync	Commission and revenue aggregation logic duplicated.	Extract shared CommissionCalculator or query helper.
CustomerService.GetCustomerProfileAsync	calculatedTotal loop duplicates GetTotalAmountByCustomerAsync / repository logic.	Use a single source of truth; remove dead calculatedTotal variable.
ReportService.GenerateReportAsync vs typed generate methods	Typed methods delegate to ReportGenerator, then GenerateReportAsync re-implements routing.	One routing mechanism only (strategy registry).
ExporterFactory	Two CreateExporter overloads with parallel switch logic.	Single creation path; normalize format input to enum or value object.
Active-entity checks	Customer/merchant IsActive validation repeated in OrderService and PaymentProcessor.	Extract IEntityGuard or shared validation rules.

---

KISS Violations (Keep It Simple, Stupid)

Location	Issue	Recommended action
PaymentService.CalculateTotalRevenueAsync	Second loop loads all payments, fetches orders, adds 0m to total — dead complexity with no effect.	Remove the entire second loop.
ReportService.GetMerchantRevenueAsync	Second loop re-fetches merchant and adds 0m — misleading and wasteful.	Single filtered aggregation pass.
ReportService.GetActiveCustomerCountAsync	Fetches active customers, then re-fetches each by ID to count again.	return customers.Count or a single repository query.
ReportService.GetRevenueByCategoryAsync	Loads all orders at the end and computes fees that are discarded (_ = fee).	Remove unused loop entirely.
OrderService.GetOrderSummaryAsync	Calls GetAllAsync() and discards the result.	Remove the call.
ReportService.ExportReport	Four branches doing identical work.	One line: return exporter.ExportReport(report).
ReportService + ReportGenerator	Two layers where ReportService mostly logs and forwards.	Merge or give ReportService a clear distinct role (orchestration vs. generation).

---

YAGNI Violations (You Aren't Gonna Need It)

Location	Issue	Recommended action
FeeCalculator.CalculateDiscount	Tier-based discount API exists but PaymentProcessor inlines VIP logic instead.	Either wire it up consistently or remove until needed.
IDataExporter.SupportsStreaming	Flag is never checked by any caller.	Remove or implement streaming only when required.
IDataExporter.WriteToFile	File I/O on exporter interface; API returns content over HTTP instead.	Move file persistence to a separate service if ever needed.
BusinessConstants.CacheExpirySeconds	Defined but no caching exists anywhere.	Remove constant or implement caching with clear invalidation rules.
BusinessConstants.LogCategory*	Unused logging category constants.	Remove or adopt consistently.
PaymentService.CalculateTotalRevenueAsync	Currency-check loop that contributes nothing.	Remove speculative code paths.

---

Additional Code Smells (Cross-Cutting)

Category	Location	Issue	Recommended action
Performance	ReportGenerator.BuildDailySalesReport	N+1 queries: fetches merchant and customer per order inside a loop.	Batch-load merchants/customers into dictionaries keyed by ID.
Performance	ReportGenerator.BuildCustomerActivityReport	N+1 queries for merchant and payment per order.	Same batching approach.
Performance	ReportService.GetActiveCustomerCountAsync	O(n) redundant DB calls.	Single query.
Magic strings	PaymentProcessor, controllers	"card", "bank_transfer" as raw strings.	Use PaymentMethod enum or constants.
Magic numbers	CustomerService, OrderService	Activity thresholds (3, 10), pending limit (5) inline.	Named constants or configuration.
Coupling	Services → InMemoryDataStore	Business layer knows about in-memory storage implementation.	Repository abstraction for reports.
Dead code	CustomerService.GetCustomerProfileAsync	calculatedTotal computed but never used.	Remove.
Inconsistency	VIP discount	PaymentProcessor checks LifetimeSpend > threshold; FeeCalculator uses tier string.	Unify VIP eligibility logic.

---

What Looks Suspicious but Should Not Be Refactored (Without Cause)

These are intentional distractions or acceptable patterns for the exercise scope:

Location	Why leave it alone
PaymentService thin wrappers (GetPaymentAsync, etc.)	Simple delegation over a repository is a valid façade; extracting further adds little value.
Repository SimulateDbLatency()	Mimics I/O latency for a realistic legacy feel; not a design flaw for this exercise.
DTOs co-located with services (OrderSummaryDto, CustomerProfileDto)	Acceptable in a small codebase; moving to separate files is optional polish, not a priority.
MerchantService.GetMerchantStatsAsync structure	Clear, single-purpose method with no obvious smell — good example of clean code in the same codebase.
In-memory storage instead of EF Core	Out of scope; replacing persistence is not the goal of this exercise.
Activity level thresholds (low / medium / high)	Business rules expressed simply; only extract if they proliferate across services.

---

Suggested Refactoring Priority (60–90 Minute Exercise)

A pragmatic order candidates often follow:

1. Remove dead/redundant code — zero-behavior-change wins (CalculateTotalRevenueAsync, GetMerchantRevenueAsync, unused loops).
2. Wire up FeeCalculator — eliminates the most visible DRY violation with minimal risk.
3. Fix N+1 in ReportGenerator — clear performance/maintainability improvement.
4. Break OCP violations — report type strategy and exporter resolution.
5. Apply ISP/LSP to exporters — split IDataExporter, fix XmlDataExporter contract.
6. SRP on PaymentProcessor — extract validation and fee strategies if time remains.

---

License

This project is provided for educational and interview purposes. Use and adapt freely.