1. Code Smell Inventory PaymentProcessor does too many jobs — (Large Class / Divergent Change) if (c.status === "delinquent") { if (Date.now() - new Date(c.delinquent_since).getTime() > 90*86400000) { await db.markChurned(c.id); await sendEmail(c.email, "Account closed", "Closed due to non-payment."); await slack.post("#billing-ops", c.id + " auto-churned"); throw new Error("churned"); } else { Risk: One method owns billing validation, account lifecycle, gateway orchestration, persistence, email, Slack, and referral credits. Any billing-policy change risks unintended payment-processing regressions. Hard-coded provider dependency — (Inappropriate Intimacy) let stripeCustomerId = c.stripecustomerid; if (!stripeCustomerId) { const created = await stripe.customers.create({ email: c.email, name: c.name }); stripeCustomerId = created.id; await db.setStripeId(c.id, stripeCustomerId); } Risk: Stripe-specific identifiers and API calls are embedded directly in domain flow. Adding Adyen, Square, or GoCardless would require editing high-risk payment orchestration instead of plugging in a provider adapter. Repeated retry logic — (Duplicated Code) let attempt = 0, refund: any; while (attempt < 3) { try { refund = await stripe.refunds.create({ charge: chargeId, amount: amount ? Math.round(amount*100) : undefined }); break; } catch (err: any) { if (err.code === "rate_limit") { await new Promise(r => setTimeout(r, 1000*(attempt+1))); attempt++; continue; } throw err; } } Risk: Rate-limit behavior is duplicated between charge and refund paths, making future idempotency-keyed retries easy to implement inconsistently. The current loop can also fall through after repeated rate limits with an undefined result. Primitive obsession around money and currency — (Primitive Obsession) async charge(amount: number, currency: string, customer: any, metadata: any) { if (!customer?.id) throw new Error("missing customer"); if (amount <= 0) throw new Error("bad amount"); if (currency !== "USD" && currency !== "EUR" && currency !== "GBP") throw new Error("unsupported currency"); Risk: Money is represented as floating-point numbers and currencies as arbitrary strings. This invites rounding inconsistencies, unsupported currencies leaking into providers, and future usage-based billing bugs. Magic numbers and hidden policy — (Magic Number) if (Date.now() - new Date(c.delinquent_since).getTime() > 90*86400000) { await db.markChurned(c.id); await sendEmail(c.email, "Account closed", "Closed due to non-payment."); await slack.post("#billing-ops", c.id + " auto-churned"); throw new Error("churned"); } else { Risk: The 90-day grace period and day duration are embedded in-line with no name or policy boundary. Changing collections policy requires editing payment execution logic. Long transactional workflow — (Long Method) await db.insertPayment(c.id, amount, currency, charge.id); if (amount > 1000) await slack.post("#billing-ops", "Large charge: " + amount + " " + currency + " for " + c.name); await sendEmail(c.email, "Payment received", "Thanks for " + amount + " " + currency); if (c.referrer_id) { const ref = await db.findCustomer(c.referrer_id); Risk: Important side effects occur after the external charge succeeds, so failures in Slack/email/referral logic can make a successful charge appear failed to callers. The workflow is hard to reason about during incident response. Conditional logic blocks extension — (Switch Statements / Repeated Conditional Complexity) if (err.code === "card_declined") { await sendEmail(c.email, "Card declined", "Update your card."); await slack.post("#billing-ops", "Card declined for " + c.id); throw err; } if (err.code === "rate_limit") { await new Promise(r => setTimeout(r, 1000*(attempt+1))); attempt++; continue; } Risk: Provider-specific error codes are interpreted directly inside business flow. New processors will introduce different decline/rate-limit semantics and grow conditional branches in the most sensitive method. 2. SOLID Assessment - Single Responsibility: FAIL — one class handles validation, customer lifecycle, provider calls, retry behavior, persistence, notifications, and referral accounting. - Open/Closed: FAIL — adding payment providers or retry/idempotency behavior requires modifying the core payment method. - Liskov Substitution: WEAK — no abstraction exists today, so substitutability is not broken yet, but provider behavior cannot be substituted safely. - Interface Segregation: WEAK — dependencies are implicit globals rather than interfaces; consumers are not forced through oversized interfaces, but collaborators are not explicit. - Dependency Inversion: FAIL — high-level billing orchestration depends directly on concrete globals like stripe, db, sendEmail, and slack. 3. Target Architecture The end state keeps PaymentProcessor as the public façade for backwards compatibility while moving volatile concerns behind narrow interfaces. The processor should orchestrate the billing use case only: validate input, load customer/payment records, invoke a payment gateway, persist results, and trigger notification/referral policies. Provider-specific behavior moves into PaymentGateway adapters such as StripePaymentGateway, later extended by Adyen, Square, and GoCardless without editing orchestration. Retry policy is centralized in one helper so idempotency-keyed retries can be added once and shared by charge, refund, and capture. Database access remains behind the existing db surface for low-downtime migration, but the constructor accepts dependencies to enable characterization tests and gradual production rollout. Notifications are grouped behind a notification service boundary to isolate operational side effects. Patterns introduced: Adapter for payment processors, Facade to preserve the current PaymentProcessor API, and Policy/Strategy for retry behavior and future provider selection. External callers | v PaymentProcessor facade | +--> Billing validation / orchestration +--> Retry policy +--> Notification service +--> Billing database boundary | v PaymentGateway interface | +--> StripePaymentGateway +--> Future: Adyen / Square / GoCardless 4. Refactoring Plan Step 1 — Extract Method and Replace Magic Values with Named Constants - Goal: Isolate charge-request validation and name supported currencies before changing payment flow. - Risk: SAFE - Precondition: Characterization tests cover missing customer, bad amount, unsupported currency, and valid USD/EUR/GBP charge paths. - Before: async charge(amount: number, currency: string, customer: any, metadata: any) { if (!customer?.id) throw new Error("missing customer"); if (amount <= 0) throw new Error("bad amount"); if (currency !== "USD" && currency !== "EUR" && currency !== "GBP") throw new Error("unsupported currency"); - After: const SUPPORTED_CURRENCIES = new Set(["USD", "EUR", "GBP"]); export class PaymentProcessor { async charge(amount: number, currency: string, customer: any, metadata: any) { this.validateChargeRequest(amount, currency, customer); } private validateChargeRequest(amount: number, currency: string, customer: any): void { if (!customer?.id) throw new Error("missing customer"); if (amount <= 0) throw new Error("bad amount"); if (!SUPPORTED_CURRENCIES.has(currency)) throw new Error("unsupported currency"); } } - Independent deployability: This changes structure only; the same inputs still throw the same error messages. Step 2 — Extract Rate-Limit Retry Policy - Goal: Remove duplicated retry loops and make rate-limit behavior consistent across charge and refund. - Risk: MEDIUM - Precondition: Tests simulate two rate_limit errors followed by success for both charge and refund, and verify non-rate-limit errors are rethrown. - Before: let attempt = 0, charge: any; while (attempt < 3) { try { charge = await stripe.charges.create({ amount: Math.round(amount*100), currency: currency.toLowerCase(), customer: stripeCustomerId, metadata }); break; } catch (err: any) { if (err.code === "card_declined") { await sendEmail(c.email, "Card declined", "Update your card."); await slack.post("#billing-ops", "Card declined for " + c.id); throw err; } if (err.code === "rate_limit") { await new Promise(r => setTimeout(r, 1000*(attempt+1))); attempt++; continue; } throw err; } } - After: const charge = await this.withRateLimitRetry(async () => { try { return await stripe.charges.create({ amount: Math.round(amount * 100), currency: currency.toLowerCase(), customer: stripeCustomerId, metadata }); } catch (err: any) { if (err.code === "card_declined") { await sendEmail(c.email, "Card declined", "Update your card."); await slack.post("#billing-ops", "Card declined for " + c.id); } throw err; } }); private async withRateLimitRetry<T>(operation: () => Promise<T>): Promise<T> { let lastError: any; for (let attempt = 0; attempt < 3; attempt++) { try { return await operation(); } catch (err: any) { lastError = err; if (err.code !== "rate_limit") { throw err; } if (attempt < 2) { await new Promise(resolve => setTimeout(resolve, 1000 * (attempt + 1))); } } } throw lastError; } - Independent deployability: The retry count and backoff schedule remain equivalent for successful retries, while the exhausted-retry case now fails explicitly instead of continuing with undefined. Step 3 — Extract Notification and Delinquency Handling - Goal: Separate customer lifecycle notifications from the payment provider call path. - Risk: MEDIUM - Precondition: Tests cover delinquent customer under 90 days receiving overdue email, over 90 days being marked churned, Slack notification, and "churned" error. - Before: if (c.status === "delinquent") { if (Date.now() - new Date(c.delinquent_since).getTime() > 90*86400000) { await db.markChurned(c.id); await sendEmail(c.email, "Account closed", "Closed due to non-payment."); await slack.post("#billing-ops", c.id + " auto-churned"); throw new Error("churned"); } else { await sendEmail(c.email, "Payment overdue", "Update your payment method."); } } - After: await this.handleDelinquentCustomer(c); private async handleDelinquentCustomer(customer: any): Promise<void> { if (customer.status !== "delinquent") { return; } if (this.isPastDelinquencyGracePeriod(customer)) { await db.markChurned(customer.id); await this.notifications.notifyAutoChurned(customer); throw new Error("churned"); } await this.notifications.notifyPaymentOverdue(customer); } private isPastDelinquencyGracePeriod(customer: any): boolean { return Date.now() - new Date(customer.delinquent_since).getTime() > 90 * 86400000; } - Independent deployability: It preserves ordering and messages while moving notification calls behind a testable boundary. Step 4 — Introduce Payment Gateway Adapter - Goal: Decouple payment orchestration from Stripe-specific customer, charge, and refund APIs. - Risk: MEDIUM - Precondition: Tests assert that missing stripecustomerid creates a Stripe customer and persists the ID before charging. - Before: let stripeCustomerId = c.stripecustomerid; if (!stripeCustomerId) { const created = await stripe.customers.create({ email: c.email, name: c.name }); stripeCustomerId = created.id; await db.setStripeId(c.id, stripeCustomerId); } - After: const gatewayCustomerId = await this.findOrCreateGatewayCustomerId(c); private async findOrCreateGatewayCustomerId(customer: any): Promise<string> { let gatewayCustomerId = customer.stripecustomerid; if (!gatewayCustomerId) { gatewayCustomerId = await this.paymentGateway.createCustomer({ email: customer.email, name: customer.name }); await db.setStripeId(customer.id, gatewayCustomerId); } return gatewayCustomerId; } interface PaymentGateway { createCustomer(customer: { email: string; name: string }): Promise<string>; charge(params: { amountInMinorUnits: number; currency: string; customerId: string; metadata: any; }): Promise<any>; refund(params: { chargeId: string; amountInMinorUnits?: number; }): Promise<any>; } class StripePaymentGateway implements PaymentGateway { async createCustomer(customer: { email: string; name: string }): Promise<string> { const created = await stripe.customers.create({ email: customer.email, name: customer.name }); return created.id; } async charge(params: { amountInMinorUnits: number; currency: string; customerId: string; metadata: any; }): Promise<any> { return stripe.charges.create({ amount: params.amountInMinorUnits, currency: params.currency.toLowerCase(), customer: params.customerId, metadata: params.metadata }); } async refund(params: { chargeId: string; amountInMinorUnits?: number; }): Promise<any> { return stripe.refunds.create({ charge: params.chargeId, amount: params.amountInMinorUnits }); } } - Independent deployability: The default adapter still calls Stripe exactly as before, but the core class no longer needs direct Stripe calls. Step 5 — Preserve Public Facade While Extracting Refund/Capture Paths - Goal: Keep public method signatures stable while routing refund and future capture behavior through shared validation, gateway, and retry boundaries. - Risk: HIGH - Precondition: Integration tests cover full refund, partial refund, missing payment, customer email on refund, and exhausted rate-limit retries. - Before: async refund(chargeId: string, amount?: number) { const p = await db.findPaymentByCharge(chargeId); if (!p) throw new Error("payment not found"); let attempt = 0, refund: any; while (attempt < 3) { try { refund = await stripe.refunds.create({ charge: chargeId, amount: amount ? Math.round(amount*100) : undefined }); break; } catch (err: any) { if (err.code === "rate_limit") { await new Promise(r => setTimeout(r, 1000*(attempt+1))); attempt++; continue; } throw err; } } await db.insertRefund(p.id, amount || p.amount, refund.id); const cust = await db.findCustomer(p.customer_id); if (cust) await sendEmail(cust.email, "Refund processed", "Refund of " + (amount || p.amount) + " issued."); return refund; } - After: async refund(chargeId: string, amount?: number) { const payment = await db.findPaymentByCharge(chargeId); if (!payment) throw new Error("payment not found"); const refund = await this.withRateLimitRetry(() => this.paymentGateway.refund({ chargeId, amountInMinorUnits: amount ? Math.round(amount * 100) : undefined }) ); const refundAmount = amount || payment.amount; await db.insertRefund(payment.id, refundAmount, refund.id); const customer = await db.findCustomer(payment.customer_id); if (customer) { await this.notifications.notifyRefundProcessed(customer, refundAmount); } return refund; } async chargeCard(amount: number, currency: string, customer: any, metadata: any) { return this.charge(amount, currency, customer, metadata); } async capturePartial(chargeId: string, amount: number) { if (!chargeId) throw new Error("missing charge"); if (amount <= 0) throw new Error("bad amount"); return this.withRateLimitRetry(() => this.paymentGateway.capturePartial(chargeId, Math.round(amount * 100)) ); } - Independent deployability: Existing refund behavior remains available, while chargeCard and capturePartial provide stable façade methods for callers without changing existing charge consumers. 5. Risk Map & Testing Strategy Characterization Tests to Write First 1. Charging rejects missing customer, non-positive amount, and unsupported currency with the current exact error messages. 2. A delinquent customer older than 90 days is marked churned, receives the account-closed email, posts to billing Slack, and throws "churned" before provider charge. 3. A Stripe card_declined error sends the card-declined email, posts to billing Slack, and rethrows the original provider error without inserting a payment. 4. Refund retries on two rate_limit errors, succeeds on the third attempt, inserts the refund, and emails the customer. Rollout Approach - Ship behind dependency-injection defaults first; production uses the existing db, stripe, sendEmail, and slack globals with no caller migration. - Canary the refactored processor to one internal billing job or low-volume tenant cohort before enabling for all live charges. - Add structured logs around retry attempts, card declines, churn decisions, and gateway adapter operations before expanding rollout. - Keep old public method names active during rollout and monitor charge p95 latency, refund failures, duplicate charges, and notification failures. Effort per Step - Step 1: XS - Step 2: S - Step 3: M - Step 4: M - Step 5: L 6. Final Refactored File declare const db: any; declare const stripe: any; declare function sendEmail(to: string, subject: string, body: string): Promise<void>; declare const slack: { post(channel: string, message: string): Promise<void>; }; type SupportedCurrency = "USD" | "EUR" | "GBP"; const SUPPORTED_CURRENCIES = new Set<string>(["USD", "EUR", "GBP"]); const DELINQUENCYGRACEPERIOD_DAYS = 90; const MILLISECONDSPERDAY = 86400000; const MAXRATELIMIT_ATTEMPTS = 3; const RATELIMITBACKOFF_MS = 1000; const LARGECHARGETHRESHOLD = 1000; const REFERRALCREDITRATE = 0.1; const BILLINGOPSCHANNEL = "#billing-ops"; interface BillingDatabase { findCustomer(customerId: string): Promise<any>; markChurned(customerId: string): Promise<void>; setStripeId(customerId: string, stripeCustomerId: string): Promise<void>; insertPayment(customerId: string, amount: number, currency: string, chargeId: string): Promise<void>; findPaymentByCharge(chargeId: string): Promise<any>; insertRefund(paymentId: string, amount: number, refundId: string): Promise<void>; addCredit(customerId: string, amount: number): Promise<void>; } interface PaymentGateway { createCustomer(customer: { email: string; name: string }): Promise<string>; charge(params: { amountInMinorUnits: number; currency: string; customerId: string; metadata: any; }): Promise<any>; refund(params: { chargeId: string; amountInMinorUnits?: number; }): Promise<any>; capturePartial(chargeId: string, amountInMinorUnits: number): Promise<any>; } interface NotificationService { notifyAutoChurned(customer: any): Promise<void>; notifyPaymentOverdue(customer: any): Promise<void>; notifyCardDeclined(customer: any): Promise<void>; notifyLargeCharge(customer: any, amount: number, currency: string): Promise<void>; notifyPaymentReceived(customer: any, amount: number, currency: string): Promise<void>; notifyReferralCredit(customer: any, credit: number, currency: string): Promise<void>; notifyRefundProcessed(customer: any, amount: number): Promise<void>; } interface PaymentProcessorDependencies { dbClient?: BillingDatabase; paymentGateway?: PaymentGateway; notificationService?: NotificationService; now?: () => number; sleep?: (milliseconds: number) => Promise<void>; } class StripePaymentGateway implements PaymentGateway { constructor(private readonly stripeClient: any = stripe) {} async createCustomer(customer: { email: string; name: string }): Promise<string> { const created = await this.stripeClient.customers.create({ email: customer.email, name: customer.name }); return created.id; } async charge(params: { amountInMinorUnits: number; currency: string; customerId: string; metadata: any; }): Promise<any> { return this.stripeClient.charges.create({ amount: params.amountInMinorUnits, currency: params.currency.toLowerCase(), customer: params.customerId, metadata: params.metadata }); } async refund(params: { chargeId: string; amountInMinorUnits?: number; }): Promise<any> { return this.stripeClient.refunds.create({ charge: params.chargeId, amount: params.amountInMinorUnits }); } async capturePartial(chargeId: string, amountInMinorUnits: number): Promise<any> { if (!this.stripeClient.charges?.capture) { throw new Error("partial capture unsupported"); } return this.stripeClient.charges.capture(chargeId, { amount: amountInMinorUnits }); } } class DefaultNotificationService implements NotificationService { constructor( private readonly emailSender: (to: string, subject: string, body: string) => Promise<void> = sendEmail, private readonly slackClient: { post(channel: string, message: string): Promise<void> } = slack ) {} async notifyAutoChurned(customer: any): Promise<void> { await this.emailSender(customer.email, "Account closed", "Closed due to non-payment."); await this.slackClient.post(BILLINGOPSCHANNEL, customer.id + " auto-churned"); } async notifyPaymentOverdue(customer: any): Promise<void> { await this.emailSender(customer.email, "Payment overdue", "Update your payment method."); } async notifyCardDeclined(customer: any): Promise<void> { await this.emailSender(customer.email, "Card declined", "Update your card."); await this.slackClient.post(BILLINGOPSCHANNEL, "Card declined for " + customer.id); } async notifyLargeCharge(customer: any, amount: number, currency: string): Promise<void> { await this.slackClient.post( BILLINGOPSCHANNEL, "Large charge: " + amount + " " + currency + " for " + customer.name ); } async notifyPaymentReceived(customer: any, amount: number, currency: string): Promise<void> { await this.emailSender(customer.email, "Payment received", "Thanks for " + amount + " " + currency); } async notifyReferralCredit(customer: any, credit: number, currency: string): Promise<void> { await this.emailSender(customer.email, "Referral credit", "You earned " + credit + " " + currency); } async notifyRefundProcessed(customer: any, amount: number): Promise<void> { await this.emailSender(customer.email, "Refund processed", "Refund of " + amount + " issued."); } } export class PaymentProcessor { private readonly dbClient: BillingDatabase; private readonly paymentGateway: PaymentGateway; private readonly notifications: NotificationService; private readonly now: () => number; private readonly sleep: (milliseconds: number) => Promise<void>; constructor(dependencies: PaymentProcessorDependencies = {}) { this.dbClient = dependencies.dbClient ?? db; this.paymentGateway = dependencies.paymentGateway ?? new StripePaymentGateway(); this.notifications = dependencies.notificationService ?? new DefaultNotificationService(); this.now = dependencies.now ?? (() => Date.now()); this.sleep = dependencies.sleep ?? ((milliseconds: number) => new Promise(resolve => setTimeout(resolve, milliseconds))); } async charge(amount: number, currency: string, customer: any, metadata: any) { return this.chargeCard(amount, currency, customer, metadata); } async chargeCard(amount: number, currency: string, customer: any, metadata: any) { this.validateChargeRequest(amount, currency, customer); const persistedCustomer = await this.dbClient.findCustomer(customer.id); if (!persistedCustomer) throw new Error("customer not found"); await this.handleDelinquentCustomer(persistedCustomer); const gatewayCustomerId = await this.findOrCreateGatewayCustomerId(persistedCustomer); const charge = await this.withRateLimitRetry(async () => { try { return await this.paymentGateway.charge({ amountInMinorUnits: this.toMinorUnits(amount), currency, customerId: gatewayCustomerId, metadata }); } catch (err: any) { if (err.code === "card_declined") { await this.notifications.notifyCardDeclined(persistedCustomer); } throw err; } }); await this.dbClient.insertPayment(persistedCustomer.id, amount, currency, charge.id); if (amount > LARGECHARGETHRESHOLD) { await this.notifications.notifyLargeCharge(persistedCustomer, amount, currency); } await this.notifications.notifyPaymentReceived(persistedCustomer, amount, currency); await this.applyReferralCreditIfNeeded(persistedCustomer, amount, currency); return charge; } async refund(chargeId: string, amount?: number) { const payment = await this.dbClient.findPaymentByCharge(chargeId); if (!payment) throw new Error("payment not found"); const refund = await this.withRateLimitRetry(() => this.paymentGateway.refund({ chargeId, amountInMinorUnits: amount ? this.toMinorUnits(amount) : undefined }) ); const refundAmount = amount || payment.amount; await this.dbClient.insertRefund(payment.id, refundAmount, refund.id); const customer = await this.dbClient.findCustomer(payment.customer_id); if (customer) { await this.notifications.notifyRefundProcessed(customer, refundAmount); } return refund; } async capturePartial(chargeId: string, amount: number) { if (!chargeId) throw new Error("missing charge"); if (amount <= 0) throw new Error("bad amount"); return this.withRateLimitRetry(() => this.paymentGateway.capturePartial(chargeId, this.toMinorUnits(amount)) ); } private validateChargeRequest(amount: number, currency: string, customer: any): void { if (!customer?.id) throw new Error("missing customer"); if (amount <= 0) throw new Error("bad amount"); if (!SUPPORTED_CURRENCIES.has(currency)) throw new Error("unsupported currency"); } private async handleDelinquentCustomer(customer: any): Promise<void> { if (customer.status !== "delinquent") { return; } if (this.isPastDelinquencyGracePeriod(customer)) { await this.dbClient.markChurned(customer.id); await this.notifications.notifyAutoChurned(customer); throw new Error("churned"); } await this.notifications.notifyPaymentOverdue(customer); } private isPastDelinquencyGracePeriod(customer: any): boolean { return ( this.now() - new Date(customer.delinquent_since).getTime() > DELINQUENCYGRACEPERIODDAYS * MILLISECONDSPER_DAY ); } private async findOrCreateGatewayCustomerId(customer: any): Promise<string> { let gatewayCustomerId = customer.stripecustomerid; if (!gatewayCustomerId) { gatewayCustomerId = await this.paymentGateway.createCustomer({ email: customer.email, name: customer.name }); await this.dbClient.setStripeId(customer.id, gatewayCustomerId); } return gatewayCustomerId; } private async applyReferralCreditIfNeeded(customer: any, amount: number, currency: string): Promise<void> { if (!customer.referrer_id) { return; } const referrer = await this.dbClient.findCustomer(customer.referrer_id); if (!referrer) { return; } const credit = amount * REFERRALCREDITRATE; await this.dbClient.addCredit(customer.referrer_id, credit); await this.notifications.notifyReferralCredit(referrer, credit, currency); } private async withRateLimitRetry<T>(operation: () => Promise<T>): Promise<T> { let lastError: any; for (let attempt = 0; attempt < MAXRATELIMIT_ATTEMPTS; attempt++) { try { return await operation(); } catch (err: any) { lastError = err; if (err.code !== "rate_limit") { throw err; } if (attempt < MAXRATELIMIT_ATTEMPTS - 1) { await this.sleep(RATELIMITBACKOFF_MS * (attempt + 1)); } } } throw lastError; } private toMinorUnits(amount: number): number { return Math.round(amount * 100); } } 7. What I Deliberately Did Not Change - Existing error messages — external services and internal modules may already branch on these strings. - Existing database method names and schema fields — changing stripecustomerid storage requires a separate migration with more than four hours of writer-downtime risk. - Existing synchronous notification behavior — moving email/Slack to an outbox would improve latency but changes failure semantics and should be rolled out separately. - Existing money representation as number — replacing it with a Money value object is desirable for usage billing but would be a broader API change.