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.
npxskills add github/awesome-copilot--skill refactorLoading…
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.
Use this skill when:
- 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
# 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 };
+ }
# 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);
+ }
# 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) { /* ... */ }
+ }
# 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('[email protected]')
+ .password('secure123')
+ .name('Test User')
+ .address(address)
+ .build();
# 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);
+ }
+ }
# BAD: Using primitives for domain concepts
- function sendEmail(to, subject, body) { /* ... */ }
- sendEmail('[email protected]', '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('[email protected]');
+ const phone = new PhoneNumber('1', '555-1234');
# 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);
# 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));
+ }
# BAD: Unused code lingers
- function oldImplementation() { /* ... */ }
- const DEPRECATED_VALUE = 5;
- import { unusedThing } from './somewhere';
- // Commented out code
- // function oldCode() { /* ... */ }
# GOOD: Remove it
+ // Delete unused functions, imports, and commented code
+ // If you need it again, git history has it
# BAD: One class reaches deep into another
- class OrderProcessor {
- process(order) {
- order.user.profile.address.street; // Too intimate
- order.repository.connection.config; // Breaking encapsulation
+ }
+ }
# GOOD: Ask, don't tell
+ class OrderProcessor {
+ process(order) {
+ order.getShippingAddress(); // Order knows how to get it
+ order.save(); // Order knows how to save itself
+ }
+ }
# Before: One long function
- function printReport(users) {
- console.log('USER REPORT');
- console.log('============');
- console.log('');
- console.log(`Total users: ${users.length}`);
- console.log('');
- console.log('ACTIVE USERS');
- console.log('------------');
- const active = users.filter(u => u.isActive);
- active.forEach(u => {
- console.log(`- ${u.name} (${u.email})`);
- });
- console.log('');
- console.log(`Active: ${active.length}`);
- console.log('');
- console.log('INACTIVE USERS');
- console.log('--------------');
- const inactive = users.filter(u => !u.isActive);
- inactive.forEach(u => {
- console.log(`- ${u.name} (${u.email})`);
- });
- console.log('');
- console.log(`Inactive: ${inactive.length}`);
- }
# After: Extracted methods
+ function printReport(users) {
+ printHeader('USER REPORT');
+ console.log(`Total users: ${users.length}\n`);
+ printUserSection('ACTIVE USERS', users.filter(u => u.isActive));
+ printUserSection('INACTIVE USERS', users.filter(u => !u.isActive));
+ }
+
+ function printHeader(title) {
+ const line = '='.repeat(title.length);
+ console.log(title);
+ console.log(line);
+ console.log('');
+ }
+
+ function printUserSection(title, users) {
+ console.log(title);
+ console.log('-'.repeat(title.length));
+ users.forEach(u => console.log(`- ${u.name} (${u.email})`));
+ console.log('');
+ console.log(`${title.split(' ')[0]}: ${users.length}`);
+ console.log('');
+ }
# Before: No types
- function calculateDiscount(user, total, membership, date) {
- if (membership === 'gold' && date.getDay() === 5) {
- return total * 0.25;
- }
- if (membership === 'gold') return total * 0.2;
- return total * 0.1;
- }
# After: Full type safety
+ type Membership = 'bronze' | 'silver' | 'gold';
+
+ interface User {
+ id: string;
+ name: string;
+ membership: Membership;
+ }
+
+ interface DiscountResult {
+ original: number;
+ discount: number;
+ final: number;
+ rate: number;
+ }
+
+ function calculateDiscount(
+ user: User,
+ total: number,
+ date: Date = new Date()
+ ): DiscountResult {
+ if (total < 0) throw new Error('Total cannot be negative');
+
+ let rate = 0.1; // Default bronze
+
+ if (user.membership === 'gold' && date.getDay() === 5) {
+ rate = 0.25; // Friday bonus for gold
+ } else if (user.membership === 'gold') {
+ rate = 0.2;
+ } else if (user.membership === 'silver') {
+ rate = 0.15;
+ }
+
+ const discount = total * rate;
+
+ return {
+ original: total,
+ discount,
+ final: total - discount,
+ rate
+ };
+ }
# Before: Conditional logic
- function calculateShipping(order, method) {
- if (method === 'standard') {
- return order.total > 50 ? 0 : 5.99;
- } else if (method === 'express') {
- return order.total > 100 ? 9.99 : 14.99;
+ } else if (method === 'overnight') {
+ return 29.99;
+ }
+ }
# After: Strategy pattern
+ interface ShippingStrategy {
+ calculate(order: Order): number;
+ }
+
+ class StandardShipping implements ShippingStrategy {
+ calculate(order: Order) {
+ return order.total > 50 ? 0 : 5.99;
+ }
+ }
+
+ class ExpressShipping implements ShippingStrategy {
+ calculate(order: Order) {
+ return order.total > 100 ? 9.99 : 14.99;
+ }
+ }
+
+ class OvernightShipping implements ShippingStrategy {
+ calculate(order: Order) {
+ return 29.99;
+ }
+ }
+
+ function calculateShipping(order: Order, strategy: ShippingStrategy) {
+ return strategy.calculate(order);
+ }
# Before: Nested validation
- function validate(user) {
- const errors = [];
- if (!user.email) errors.push('Email required');
+ else if (!isValidEmail(user.email)) errors.push('Invalid email');
+ if (!user.name) errors.push('Name required');
+ if (user.age < 18) errors.push('Must be 18+');
+ if (user.country === 'blocked') errors.push('Country not supported');
+ return errors;
+ }
# After: Chain of responsibility
+ abstract class Validator {
+ abstract validate(user: User): string | null;
+ setNext(validator: Validator): Validator {
+ this.next = validator;
+ return validator;
+ }
+ validate(user: User): string | null {
+ const error = this.doValidate(user);
+ if (error) return error;
+ return this.next?.validate(user) ?? null;
+ }
+ }
+
+ class EmailRequiredValidator extends Validator {
+ doValidate(user: User) {
+ return !user.email ? 'Email required' : null;
+ }
+ }
+
+ class EmailFormatValidator extends Validator {
+ doValidate(user: User) {
+ return user.email && !isValidEmail(user.email) ? 'Invalid email' : null;
+ }
+ }
+
+ // Build the chain
+ const validator = new EmailRequiredValidator()
+ .setNext(new EmailFormatValidator())
+ .setNext(new NameRequiredValidator())
+ .setNext(new AgeValidator())
+ .setNext(new CountryValidator());
1. PREPARE
- Ensure tests exist (write them if missing)
- Commit current state
- Create feature branch
2. IDENTIFY
- Find the code smell to address
- Understand what the code does
- Plan the refactoring
3. REFACTOR (small steps)
- Make one small change
- Run tests
- Commit if tests pass
- Repeat
4. VERIFY
- All tests pass
- Manual testing if needed
- Performance unchanged or improved
5. CLEAN UP
- Update comments
- Update documentation
- Final commit
any types without justification| Operation | Description |
|---|---|
| Extract Method | Turn code fragment into method |
| Extract Class | Move behavior to new class |
| Extract Interface | Create interface from implementation |
| Inline Method | Move method body back to caller |
| Inline Class | Move class behavior to caller |
| Pull Up Method | Move method to superclass |
| Push Down Method | Move method to subclass |
| Rename Method/Variable | Improve clarity |
| Introduce Parameter Object | Group related parameters |
| Replace Conditional with Polymorphism | Use polymorphism instead of switch/if |
| Replace Magic Number with Constant | Named constants |
| Decompose Conditional | Break complex conditions |
| Consolidate Conditional | Combine duplicate conditions |
| Replace Nested Conditional with Guard Clauses | Early returns |
| Introduce Null Object | Eliminate null checks |
| Replace Type Code with Class/Enum | Strong typing |
| Replace Inheritance with Delegation | Composition over inheritance |
Create or update AgentSkills. Use when designing, structuring, or packaging skills with scripts, references, and assets.
Create or update AgentSkills. Use when designing, structuring, or packaging skills with scripts, references, and assets.
Set up and use 1Password CLI (op). Use when installing the CLI, enabling desktop app integration, signing in (single or multi-account), or reading/injecting/running secrets via op.
CLI to manage emails via IMAP/SMTP. Use `himalaya` to list, read, write, reply, forward, search, and organize emails from the terminal. Supports multiple accounts and message composition with MML (MIME Meta Language).
Create or update AgentSkills. Use when designing, structuring, or packaging skills with scripts, references, and assets.
Create or update AgentSkills. Use when designing, structuring, or packaging skills with scripts, references, and assets.
Set up and use 1Password CLI (op). Use when installing the CLI, enabling desktop app integration, signing in (single or multi-account), or reading/injecting/running secrets via op.
CLI to manage emails via IMAP/SMTP. Use `himalaya` to list, read, write, reply, forward, search, and organize emails from the terminal. Supports multiple accounts and message composition with MML (MIME Meta Language).