name: refactor description: 'Surgical code refactoring to improve maintainability without changing behavior. Covers extracting functions, renaming variables, breaking down god functions, improving type safety, eliminating code smells, and applying design patterns. Less drastic than repo-rebuilder; use for gradual improvements.' license: MIT
Refactor
Overview
Improve code structure and readability without changing external behavior. Refactoring is gradual evolution, not revolution. Use this for improving existing code, not rewriting from scratch.
When to Use
Use this skill when:
- Code is hard to understand or maintain
- Functions/classes are too large
- Code smells need addressing
- Adding features is difficult due to code structure
- User asks "clean up this code", "refactor this", "improve this"
Refactoring Principles
The Golden Rules
- Behavior is preserved - Refactoring doesn't change what the code does, only how
- Small steps - Make tiny changes, test after each
- Version control is your friend - Commit before and after each safe state
- Tests are essential - Without tests, you're not refactoring, you're editing
- One thing at a time - Don't mix refactoring with feature changes
When NOT to Refactor
- Code that works and won't change again (if it ain't broke...)
- Critical production code without tests (add tests first)
- When you're under a tight deadline
- "Just because" - need a clear purpose
Common Code Smells & Fixes
1. Long Method/Function
# BAD: 200-line function that does everything
- async function processOrder(orderId) {
- // 50 lines: fetch order
- // 30 lines: validate order
- // 40 lines: calculate pricing
- // 30 lines: update inventory
- // 20 lines: create shipment
- // 30 lines: send notifications
- }
# GOOD: Broken into focused functions
+ async function processOrder(orderId) {
+ const order = await fetchOrder(orderId);
+ validateOrder(order);
+ const pricing = calculatePricing(order);
+ await updateInventory(order);
+ const shipment = await createShipment(order);
+ await sendNotifications(order, pricing, shipment);
+ return { order, pricing, shipment };
+ }
2. Duplicated Code
# BAD: Same logic in multiple places
- function calculateUserDiscount(user) {
- if (user.membership === 'gold') return user.total * 0.2;
- if (user.membership === 'silver') return user.total * 0.1;
- return 0;
- }
-
- function calculateOrderDiscount(order) {
- if (order.user.membership === 'gold') return order.total * 0.2;
- if (order.user.membership === 'silver') return order.total * 0.1;
- return 0;
- }
# GOOD: Extract common logic
+ function getMembershipDiscountRate(membership) {
+ const rates = { gold: 0.2, silver: 0.1 };
+ return rates[membership] || 0;
+ }
+
+ function calculateUserDiscount(user) {
+ return user.total * getMembershipDiscountRate(user.membership);
+ }
+
+ function calculateOrderDiscount(order) {
+ return order.total * getMembershipDiscountRate(order.user.membership);
+ }
3. Large Class/Module
# BAD: God object that knows too much
- class UserManager {
- createUser() { /* ... */ }
- updateUser() { /* ... */ }
- deleteUser() { /* ... */ }
- sendEmail() { /* ... */ }
- generateReport() { /* ... */ }
- handlePayment() { /* ... */ }
- validateAddress() { /* ... */ }
- // 50 more methods...
- }
# GOOD: Single responsibility per class
+ class UserService {
+ create(data) { /* ... */ }
+ update(id, data) { /* ... */ }
+ delete(id) { /* ... */ }
+ }
+
+ class EmailService {
+ send(to, subject, body) { /* ... */ }
+ }
+
+ class ReportService {
+ generate(type, params) { /* ... */ }
+ }
+
+ class PaymentService {
+ process(amount, method) { /* ... */ }
+ }
4. Long Parameter List
# BAD: Too many parameters
- function createUser(email, password, name, age, address, city, country, phone) {
- /* ... */
- }
# GOOD: Group related parameters
+ interface UserData {
+ email: string;
+ password: string;
+ name: string;
+ age?: number;
+ address?: Address;
+ phone?: string;
+ }
+
+ function createUser(data: UserData) {
+ /* ... */
+ }
# EVEN BETTER: Use builder pattern for complex construction
+ const user = UserBuilder
+ .email('test@example.com')
+ .password('secure123')
+ .name('Test User')
+ .address(address)
+ .build();
5. Feature Envy
# BAD: Method that uses another object's data more than its own
- class Order {
- calculateDiscount(user) {
- if (user.membershipLevel === 'gold') {
+ return this.total * 0.2;
+ }
+ if (user.accountAge > 365) {
+ return this.total * 0.1;
+ }
+ return 0;
+ }
+ }
# GOOD: Move logic to the object that owns the data
+ class User {
+ getDiscountRate(orderTotal) {
+ if (this.membershipLevel === 'gold') return 0.2;
+ if (this.accountAge > 365) return 0.1;
+ return 0;
+ }
+ }
+
+ class Order {
+ calculateDiscount(user) {
+ return this.total * user.getDiscountRate(this.total);
+ }
+ }
6. Primitive Obsession
# BAD: Using primitives for domain concepts
- function sendEmail(to, subject, body) { /* ... */ }
- sendEmail('user@example.com', 'Hello', '...');
- function createPhone(country, number) {
- return `${country}-${number}`;
- }
# GOOD: Use domain types
+ class Email {
+ private constructor(public readonly value: string) {
+ if (!Email.isValid(value)) throw new Error('Invalid email');
+ }
+ static create(value: string) { return new Email(value); }
+ static isValid(email: string) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); }
+ }
+
+ class PhoneNumber {
+ constructor(
+ public readonly country: string,
+ public readonly number: string
+ ) {
+ if (!PhoneNumber.isValid(country, number)) throw new Error('Invalid phone');
+ }
+ toString() { return `${this.country}-${this.number}`; }
+ static isValid(country: string, number: string) { /* ... */ }
+ }
+
+ // Usage
+ const email = Email.create('user@example.com');
+ const phone = new PhoneNumber('1', '555-1234');
7. Magic Numbers/Strings
# BAD: Unexplained values
- if (user.status === 2) { /* ... */ }
- const discount = total * 0.15;
- setTimeout(callback, 86400000);
# GOOD: Named constants
+ const UserStatus = {
+ ACTIVE: 1,
+ INACTIVE: 2,
+ SUSPENDED: 3
+ } as const;
+
+ const DISCOUNT_RATES = {
+ STANDARD: 0.1,
+ PREMIUM: 0.15,
+ VIP: 0.2
+ } as const;
+
+ const ONE_DAY_MS = 24 * 60 * 60 * 1000;
+
+ if (user.status === UserStatus.INACTIVE) { /* ... */ }
+ const discount = total * DISCOUNT_RATES.PREMIUM;
+ setTimeout(callback, ONE_DAY_MS);
8. Nested Conditionals
# BAD: Arrow code
- function process(order) {
- if (order) {
- if (order.user) {
- if (order.user.isActive) {
- if (order.total > 0) {
- return processOrder(order);
+ } else {
+ return { error: 'Invalid total' };
+ }
+ } else {
+ return { error: 'User inactive' };
+ }
+ } else {
+ return { error: 'No user' };
+ }
+ } else {
+ return { error: 'No order' };
+ }
+ }
# GOOD: Guard clauses / early returns
+ function process(order) {
+ if (!order) return { error: 'No order' };
+ if (!order.user) return { error: 'No user' };
+ if (!order.user.isActive) return { error: 'User inactive' };
+ if (order.total <= 0) return { error: 'Invalid total' };
+ return processOrder(order);
+ }
# EVEN BETTER: Using Result type
+ function process(order): Result<ProcessedOrder, Error> {
+ return Result.combine([
+ validateOrderExists(order),
+ validateUserExists(order),
+ validateUserActive(order.user),
+ validateOrderTotal(order)
+ ]).flatMap(() => processOrder(order));
+ }
9. Dead Code
# BAD: U