---
name: vm-code-refactor
description: Intelligent code refactoring based on best practices. Detects code smells, applies design patterns, improves readability, enforces SOLID principles, and optimizes performance. Auto-detects framework and generates comprehensive refactoring plans.
---
# VM Code Refactor Skill
Comprehensive code refactoring with automatic framework detection and best practices enforcement.
## When to Use
- "Refactor this code"
- "Clean up this file"
- "Improve code structure"
- "Apply design patterns"
- "Fix code smells"
- "Reduce technical debt"
- "Make this code more maintainable"
## Workflow
### Step 1: Initial Prompt
**ALWAYS start by asking the user:**
```
question tool parameters:
questions:
- question: "Ready to refactor your code?"
header: "Refactor"
options:
- label: "Yes, let's go! (Recommended)"
description: "Start the refactoring process"
- label: "No, thanks."
description: "Exit without changes"
```
**If "No, thanks."** ā Stop with:
```
No problem! Run this skill again when you're ready to refactor. š§
```
**If "Yes, let's go!"** ā Proceed to Step 2
### Step 2: Inspection Decision
**After user confirms "Yes, let's go!", ask:**
```
question tool parameters:
questions:
- question: "Would you like me to inspect if refactoring is needed first?"
header: "Inspect"
options:
- label: "Yes, inspect first (Recommended)"
description: "Analyze codebase for refactoring opportunities before proceeding"
- label: "Skip inspection"
description: "Go directly to scope selection if you already know what to refactor"
```
**If "Skip inspection"** ā Proceed directly to Step 3 (Scope Selection)
**If "Yes, inspect first"** ā Display:
```
Analyzing codebase for refactoring opportunities... š
```
Then prompt for inspection type:
```
question tool parameters:
questions:
- question: "What level of inspection would you like?"
header: "Detail Level"
options:
- label: "Quick Scan (Recommended)"
description: "Count-based summary: total issues by category and priority"
- label: "Detailed Report"
description: "Full analysis with file-by-file breakdown of all issues"
```
#### Quick Scan Output:
```
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
REFACTORING ANALYSIS - QUICK SCAN
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
Framework: React 18.2 + TypeScript 5.0
Files Analyzed: 47
Issues Found: 23
By Priority:
š“ Critical: 3
š” Major: 8
šµ Minor: 12
By Category:
Code Smells: 15
SOLID Violations: 5
Performance Issues: 3
Top 3 Areas Needing Attention:
1. UserManager.ts - God object pattern
2. PaymentService.ts - Tight coupling
3. API routes - N+1 query problems
Recommendation: Refactoring would significantly improve code quality.
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
Proceeding to scope selection...
```
#### Detailed Report Output:
```
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
REFACTORING ANALYSIS - DETAILED REPORT
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
Framework: React 18.2 + TypeScript 5.0
Build Tool: Vite 5.0
Linter: ESLint 8.0
Files Analyzed: 47
## Issues by File:
### src/models/UserManager.ts (Critical)
- God Object: 800+ lines, 15 methods
- Multiple Responsibilities: Auth, Permissions, Notifications, Billing
- Recommendation: Split into 4 separate service classes
### src/services/PaymentService.ts (Critical)
- Tight Coupling: Direct Stripe dependency
- No Dependency Injection
- No Error Handling
- Recommendation: Implement PaymentGateway interface
### src/api/routes/users.ts (Major)
- N+1 Query Problem: 101 queries for 100 users
- Current: 8.2s load time
- Recommendation: Use eager loading (1 query)
### src/components/UserCard.tsx (Major)
- Missing memoization
- Unnecessary re-renders on parent update
- Recommendation: Add React.memo and useCallback
### src/utils/validation.ts (Minor)
- Magic Numbers: Password length 8, email regex
- Recommendation: Extract to constants
[... continue for all files with issues ...]
## Summary:
| Category | Critical | Major | Minor | Total |
|----------|----------|-------|-------|-------|
| Code Smells | 2 | 5 | 8 | 15 |
| SOLID Violations | 1 | 3 | 1 | 5 |
| Performance | 0 | 2 | 1 | 3 |
| **Total** | **3** | **10** | **10** | **23** |
## Recommended Priority Order:
1. Decouple PaymentService (2h) - Critical
2. Break up UserManager (3h) - Critical
3. Fix N+1 queries (1h) - Major
4. Add React memoization (1h) - Major
5. Extract magic numbers (30m) - Minor
Estimated Total Time: 7.5 hours
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā
Proceeding to scope selection...
```
**After inspection output, proceed to Step 3 (Scope Selection)**
### Step 3: Scope Selection
**Prompt for refactoring scope:**
```
question tool parameters:
questions:
- question: "What would you like to refactor?"
header: "Scope"
options:
- label: "Specific file(s) - I'll provide path(s)"
description: "Select one or more files to refactor"
- label: "Entire codebase"
description: "Analyze and refactor all source files"
- label: "Cancel"
description: "Exit without making changes"
```
**Option 1: Specific file(s)**
- Ask user to provide file path(s)
- User can provide multiple paths separated by commas or newlines
- Example: `src/utils/helper.js, src/models/User.js`
**Path Validation:**
After receiving paths, validate each one:
- **All paths invalid**: Display error message and return to scope selection:
```
ā The following paths could not be found:
- src/utils/helper.js
- src/models/User.js
Please check the paths and try again.
```
Allow user to retry with corrected paths.
- **Partial paths invalid**: List invalid paths, prompt user to proceed with valid paths only:
```
ā ļø The following paths could not be found:
- src/invalid-file.js
Valid paths found:
- src/utils/helper.js ā
- src/models/User.js ā
Would you like to proceed with only the valid paths?
```
User can choose to proceed or return to scope selection to correct paths.
- **All paths valid**: Proceed with analysis immediately.
**Option 2: Entire codebase**
- Scan all source files
- Skip node_modules, dist, build, etc.
- Prioritize by complexity and impact
**Option 3: Cancel**
- Stop with: `Refactoring cancelled. No changes made. š§`
### Step 4: Generate Plan
**After Option 1 or 2, display:**
```
Creating Plan... š
Detecting framework and analyzing code...
```
**Framework Detection:**
```
Framework Detected: React 18.2 + TypeScript 5.0 ā
Build Tool: Vite 5.0
Linter: ESLint 8.0
State Management: Redux Toolkit
Analyzing code patterns and smells...
```
**Generate `REFACTOR_PLANNING.md` in project root**
**After plan creation:**
```
PLAN CREATED! š
Location: /project-root/REFACTOR_PLANNING.md
Summary:
- 23 refactoring opportunities found
- Estimated impact: High
- Estimated time: 4-6 hours
```
### Step 5: Implementation Decision
**Prompt for implementation:**
```
question tool parameters:
questions:
- question: "Do you want to continue with implementing the plan?"
header: "Implement"
options:
- label: "Yes, implement now (Recommended)"
description: "Execute the refactoring plan immediately"
- label: "No, thanks."
description: "Save the plan for manual implementation later"
```
**Option 1: Yes, implement now (Recommended)**
- Execute refactoring plan
- Create backup of original files
- Apply changes incrementally
- Run tests after each major change
- Display: `ALL DONE! š`
**Option 2: No, thanks.**
- Stop with: `Plan saved. You can implement it manually or run this skill again. š§`
## Refactoring Categories
### 1. Code Smells
**Long Method:**
```javascript
// ā Too long, does too much
function processOrder(order) {
// Validate order (20 lines)
if (!order.id) throw new Error('Invalid order');
if (!order.items || order.items.length === 0) throw new Error('No items');
// ... 15 more validation lines
// Calculate total (15 lines)
let total = 0;
for (let item of order.items) {
total += item.price * item.quantity;
if (item.discount) {
total -= item.discount;
}
}
// Apply tax (10 lines)
const taxRate = getTaxRate(order.state);
total = total * (1 + taxRate);
// Process payment (25 lines)
// ... payment processing logic
// Send confirmation (15 lines)
// ... email sending logic
return { success: true, total };
}
// ā
Refactored: Single Responsibility
function processOrder(order) {
validateOrder(order);
const subtotal = calculateSubtotal(order.items);
const total = applyTax(subtotal, order.state);
const payment = processPayment(order.paymentMethod, total);
sendConfirmation(order.email, order, payment);
return { success: true, total, paymentId: payment.id };
}
function validateOrder(order) {
if (!order.id) throw new Error('Invalid order ID');
if (!order.items?.length) throw new Error('Order has no items');
if (!order.paymentMethod) throw new Error('No payment method');
}
function calculateSubtotal(items) {
return items.reduce((sum, item) => {
const itemTotal = item.price * item.quantity;
return sum + itemTotal - (item.discount || 0);
}, 0);
}
function applyTax(amount, state) {
const taxRate = getTaxRate(state);
return amount * (1 + taxRate);
}
```
**Duplicate Code:**
```python
# ā Repeated logic
class UserService:
def create_user(self, data):
if not data.get('email'):
raise ValueError("Email required")
if not re.match(r'^[\w\.-]+@[\w\.-]+\.\w+$', data['email']):
raise ValueError("Invalid email")
if len(data.get('password', '')) < 8:
raise ValueError("Password too short")
# ... create user
def update_user(self, user_id, data):
if 'email' in data:
if not data['email']:
raise ValueError("Email required")
if not re.match(r'^[\w\.-]+@[\w\.-]+\.\w+$', data['email']):
raise ValueError("Invalid email")
if 'password' in data:
if len(data['password']) < 8:
raise ValueError("Password too short")
# ... update user
# ā
Refactored: DRY principle
class UserService:
def create_user(self, data):
self._validate_user_data(data, require_all=True)
# ... create user
def update_user(self, user_id, data):
self._validate_user_data(data, require_all=False)
# ... update user
def _validate_user_data(self, data, require_all=False):
if require_all or 'email' in data:
if not data.get('email'):
raise ValueError("Email required")
if not self._is_valid_email(data['email']):
raise ValueError("Invalid email")
if require_all or 'password' in data:
if len(data.get('password', '')) < 8:
raise ValueError("Password must be at least 8 characters")
@staticmethod
def _is_valid_email(email):
return bool(re.match(r'^[\w\.-]+@[\w\.-]+\.\w+$', email))
```
**God Object:**
```javascript
// ā Class does everything
class User {
constructor(data) { this.data = data; }
// User data management
save() { /* DB logic */ }
update() { /* DB logic */ }
delete() { /* DB logic */ }
// Authentication
login(password) { /* Auth logic */ }
logout() { /* Auth logic */ }
resetPassword() { /* Email logic */ }
// Permissions
hasPermission(resource) { /* Permission logic */ }
grantPermission(resource) { /* Permission logic */ }
// Notifications
sendEmail(message) { /* Email logic */ }
sendSMS(message) { /* SMS logic */ }
// Billing
chargeCreditCard(amount) { /* Payment logic */ }
refund(amount) { /* Payment logic */ }
}
// ā
Refactored: Separation of Concerns
class User {
constructor(data) {
this.data = data;
this.auth = new UserAuth(this);
this.permissions = new UserPermissions(this);
this.notifications = new UserNotifications(this);
this.billing = new UserBilling(this);
}
save() { return UserRepository.save(this); }
update(data) { return UserRepository.update(this.id, data); }
delete() { return UserRepository.delete(this.id); }
}
class UserAuth {
constructor(user) { this.user = user; }
login(password) { /* Auth logic */ }
logout() { /* Auth logic */ }
resetPassword() { /* Auth logic */ }
}
class UserPermissions {
constructor(user) { this.user = user; }
has(resource) { /* Permission logic */ }
grant(resource) { /* Permission logic */ }
}
```
### 2. SOLID Principles
**Single Responsibility Principle:**
```typescript
// ā Multiple responsibilities
class Report {
data: any[];
constructor(data: any[]) {
this.data = data;
}
// Responsibility 1: Data processing
processData() {
return this.data.map(item => ({
...item,
total: item.price * item.quantity
}));
}
// Responsibility 2: Formatting
formatAsHTML() {
const processed = this.processData();
return `
${processed.map(row =>
`| ${row.name} | ${row.total} |
`
).join('')}
`;
}
// Responsibility 3: Export
exportToPDF() {
const html = this.formatAsHTML();
// PDF generation logic
}
// Responsibility 4: Persistence
saveToDatabase() {
const processed = this.processData();
// Database save logic
}
}
// ā
Refactored: Single responsibility per class
class ReportData {
constructor(private data: any[]) {}
process() {
return this.data.map(item => ({
...item,
total: item.price * item.quantity
}));
}
}
class ReportFormatter {
formatAsHTML(data: any[]) {
return `${data.map(row =>
`| ${row.name} | ${row.total} |
`
).join('')}
`;
}
formatAsJSON(data: any[]) {
return JSON.stringify(data, null, 2);
}
}
class ReportExporter {
exportToPDF(html: string) {
// PDF generation logic
}
exportToExcel(data: any[]) {
// Excel generation logic
}
}
class ReportRepository {
save(data: any[]) {
// Database save logic
}
}
```
**Open/Closed Principle:**
```python
# ā Modifying class to add new behavior
class PaymentProcessor:
def process(self, payment_method, amount):
if payment_method == 'credit_card':
# Credit card logic
pass
elif payment_method == 'paypal':
# PayPal logic
pass
elif payment_method == 'bitcoin': # New payment method requires modification
# Bitcoin logic
pass
# ā
Refactored: Open for extension, closed for modification
from abc import ABC, abstractmethod
class PaymentMethod(ABC):
@abstractmethod
def process(self, amount):
pass
class CreditCardPayment(PaymentMethod):
def process(self, amount):
# Credit card logic
pass
class PayPalPayment(PaymentMethod):
def process(self, amount):
# PayPal logic
pass
class BitcoinPayment(PaymentMethod): # New payment method via extension
def process(self, amount):
# Bitcoin logic
pass
class PaymentProcessor:
def process(self, payment_method: PaymentMethod, amount):
return payment_method.process(amount)
```
### 3. Design Patterns
**Strategy Pattern:**
```javascript
// ā Hard-coded sorting logic
class DataSorter {
sort(data, type) {
if (type === 'alphabetical') {
return data.sort((a, b) => a.name.localeCompare(b.name));
} else if (type === 'date') {
return data.sort((a, b) => new Date(a.date) - new Date(b.date));
} else if (type === 'price') {
return data.sort((a, b) => a.price - b.price);
}
}
}
// ā
Refactored: Strategy pattern
class SortStrategy {
sort(data) {
throw new Error('Must implement sort method');
}
}
class AlphabeticalSort extends SortStrategy {
sort(data) {
return data.sort((a, b) => a.name.localeCompare(b.name));
}
}
class DateSort extends SortStrategy {
sort(data) {
return data.sort((a, b) => new Date(a.date) - new Date(b.date));
}
}
class PriceSort extends SortStrategy {
sort(data) {
return data.sort((a, b) => a.price - b.price);
}
}
class DataSorter {
constructor(strategy) {
this.strategy = strategy;
}
setStrategy(strategy) {
this.strategy = strategy;
}
sort(data) {
return this.strategy.sort(data);
}
}
// Usage
const sorter = new DataSorter(new AlphabeticalSort());
sorter.sort(data);
sorter.setStrategy(new PriceSort());
sorter.sort(data);
```
**Factory Pattern:**
```typescript
// ā Direct instantiation everywhere
const user = new User(data);
const admin = new Admin(data);
const guest = new Guest(data);
// Repeated logic in multiple places
if (data.role === 'admin') {
return new Admin(data);
} else if (data.role === 'user') {
return new User(data);
} else {
return new Guest(data);
}
// ā
Refactored: Factory pattern
interface User {
role: string;
permissions: string[];
login(): void;
}
class RegularUser implements User {
role = 'user';
permissions = ['read'];
constructor(private data: any) {}
login() { /* User login logic */ }
}
class Admin implements User {
role = 'admin';
permissions = ['read', 'write', 'delete'];
constructor(private data: any) {}
login() { /* Admin login logic */ }
}
class Guest implements User {
role = 'guest';
permissions = [];
constructor(private data: any) {}
login() { /* Guest login logic */ }
}
class UserFactory {
static create(data: any): User {
switch (data.role) {
case 'admin':
return new Admin(data);
case 'user':
return new RegularUser(data);
default:
return new Guest(data);
}
}
}
// Usage
const user = UserFactory.create(data);
```
### 4. Performance Optimization
**Unnecessary Re-renders (React):**
```jsx
// ā Re-renders on every parent update
function UserList({ users, onDelete }) {
return (
{users.map(user => (
onDelete(user.id)} // New function every render
/>
))}
);
}
function UserCard({ user, onDelete }) {
console.log('UserCard rendered'); // Logs on every parent update
return (
{user.name}
);
}
// ā
Refactored: Memoization
import { memo, useCallback } from 'react';
function UserList({ users, onDelete }) {
const handleDelete = useCallback((userId) => {
onDelete(userId);
}, [onDelete]);
return (
{users.map(user => (
))}
);
}
const UserCard = memo(({ user, onDelete }) => {
console.log('UserCard rendered'); // Only logs when user changes
const handleClick = useCallback(() => {
onDelete(user.id);
}, [user.id, onDelete]);
return (
{user.name}
);
});
```
**N+1 Query Problem:**
```python
# ā N+1 queries
def get_users_with_posts():
users = User.objects.all() # 1 query
result = []
for user in users:
posts = user.posts.all() # N queries (one per user)
result.append({
'user': user,
'posts': posts
})
return result
# ā
Refactored: Eager loading
def get_users_with_posts():
users = User.objects.prefetch_related('posts').all() # 2 queries total
return [{
'user': user,
'posts': user.posts.all()
} for user in users]
```
**Inefficient Algorithms:**
```javascript
// ā O(n²) complexity
function findDuplicates(arr) {
const duplicates = [];
for (let i = 0; i < arr.length; i++) {
for (let j = i + 1; j < arr.length; j++) {
if (arr[i] === arr[j] && !duplicates.includes(arr[i])) {
duplicates.push(arr[i]);
}
}
}
return duplicates;
}
// ā
Refactored: O(n) complexity
function findDuplicates(arr) {
const seen = new Set();
const duplicates = new Set();
for (const item of arr) {
if (seen.has(item)) {
duplicates.add(item);
} else {
seen.add(item);
}
}
return Array.from(duplicates);
}
```
### 5. Readability Improvements
**Complex Conditionals:**
```python
# ā Hard to read nested conditions
def can_access_resource(user, resource):
if user:
if user.is_active:
if user.role == 'admin' or (user.role == 'user' and resource.owner_id == user.id):
if not resource.is_deleted:
if resource.visibility == 'public' or user.id in resource.allowed_users:
return True
return False
# ā
Refactored: Early returns and extracted conditions
def can_access_resource(user, resource):
if not user or not user.is_active:
return False
if resource.is_deleted:
return False
if user.role == 'admin':
return True
if resource.owner_id == user.id:
return True
if resource.visibility == 'public':
return True
return user.id in resource.allowed_users
```
**Magic Numbers:**
```javascript
// ā Magic numbers
function calculateDiscount(total) {
if (total > 100) {
return total * 0.1;
} else if (total > 50) {
return total * 0.05;
}
return 0;
}
setTimeout(() => {
checkStatus();
}, 86400000);
// ā
Refactored: Named constants
const DISCOUNT_TIERS = {
PREMIUM_THRESHOLD: 100,
PREMIUM_RATE: 0.1,
STANDARD_THRESHOLD: 50,
STANDARD_RATE: 0.05
};
const TIME_INTERVALS = {
ONE_DAY_MS: 24 * 60 * 60 * 1000
};
function calculateDiscount(total) {
if (total > DISCOUNT_TIERS.PREMIUM_THRESHOLD) {
return total * DISCOUNT_TIERS.PREMIUM_RATE;
} else if (total > DISCOUNT_TIERS.STANDARD_THRESHOLD) {
return total * DISCOUNT_TIERS.STANDARD_RATE;
}
return 0;
}
setTimeout(() => {
checkStatus();
}, TIME_INTERVALS.ONE_DAY_MS);
```
**Poor Naming:**
```typescript
// ā Unclear variable names
function calc(a: number, b: number): number {
const x = a * b;
const y = x * 0.08;
const z = x + y;
return z;
}
// ā
Refactored: Descriptive names
function calculateOrderTotal(price: number, quantity: number): number {
const subtotal = price * quantity;
const tax = subtotal * 0.08;
const totalWithTax = subtotal + tax;
return totalWithTax;
}
```
### 6. Error Handling
**Silent Failures:**
```python
# ā Errors silently ignored
def fetch_user_data(user_id):
try:
response = api.get(f'/users/{user_id}')
return response.json()
except:
return None # What went wrong?
# ā
Refactored: Proper error handling
class UserFetchError(Exception):
pass
def fetch_user_data(user_id):
try:
response = api.get(f'/users/{user_id}')
response.raise_for_status()
return response.json()
except requests.HTTPError as e:
if e.response.status_code == 404:
raise UserFetchError(f"User {user_id} not found")
raise UserFetchError(f"HTTP error: {e.response.status_code}")
except requests.RequestException as e:
raise UserFetchError(f"Network error: {str(e)}")
except ValueError as e:
raise UserFetchError(f"Invalid JSON response: {str(e)}")
```
**Try-Catch Overuse:**
```javascript
// ā Try-catch for control flow
function parseDate(dateString) {
try {
return new Date(dateString);
} catch {
try {
return moment(dateString).toDate();
} catch {
try {
return parseCustomFormat(dateString);
} catch {
return null;
}
}
}
}
// ā
Refactored: Validation before parsing
function parseDate(dateString) {
if (!dateString) return null;
// Try native Date first
const nativeDate = new Date(dateString);
if (!isNaN(nativeDate.getTime())) {
return nativeDate;
}
// Try moment.js
if (moment(dateString, moment.ISO_8601, true).isValid()) {
return moment(dateString).toDate();
}
// Try custom format
if (isCustomFormat(dateString)) {
return parseCustomFormat(dateString);
}
return null;
}
```
### 7. Architecture Improvements
**Tight Coupling:**
```typescript
// ā Tightly coupled
class OrderService {
processOrder(order: Order) {
// Direct dependency on specific payment gateway
const stripe = new StripeGateway(API_KEY);
const payment = stripe.charge(order.total);
// Direct dependency on specific email service
const sendgrid = new SendGridClient(API_KEY);
sendgrid.send({
to: order.email,
template: 'order_confirmation'
});
return { orderId: order.id, paymentId: payment.id };
}
}
// ā
Refactored: Dependency injection
interface PaymentGateway {
charge(amount: number): Promise;
}
interface EmailService {
send(options: EmailOptions): Promise;
}
class OrderService {
constructor(
private paymentGateway: PaymentGateway,
private emailService: EmailService
) {}
async processOrder(order: Order) {
const payment = await this.paymentGateway.charge(order.total);
await this.emailService.send({
to: order.email,
template: 'order_confirmation',
data: { orderId: order.id }
});
return { orderId: order.id, paymentId: payment.id };
}
}
// Usage with different implementations
const orderService = new OrderService(
new StripeGateway(config.stripe),
new SendGridClient(config.sendgrid)
);
// Easy to swap implementations
const testOrderService = new OrderService(
new MockPaymentGateway(),
new MockEmailService()
);
```
### 8. State Management
**Prop Drilling (React):**
```jsx
// ā Passing props through multiple levels
function App() {
const [user, setUser] = useState(null);
return ;
}
function Layout({ user, setUser }) {
return ;
}
function Sidebar({ user, setUser }) {
return ;
}
function UserMenu({ user, setUser }) {
return {user?.name}
;
}
// ā
Refactored: Context API
const UserContext = createContext();
function App() {
const [user, setUser] = useState(null);
return (
);
}
function Layout() {
return ;
}
function Sidebar() {
return ;
}
function UserMenu() {
const { user } = useContext(UserContext);
return {user?.name}
;
}
```
## REFACTOR_PLANNING.md Format
```markdown
# Refactoring Plan
**Generated**: 2026-02-14
**Scope**: Entire Codebase
**Framework**: React 18.2 + TypeScript
**Total Issues**: 23
**Estimated Time**: 4-6 hours
## Executive Summary
Current code health: **68/100** š”
**Critical Issues**: 3
**Major Issues**: 8
**Minor Issues**: 12
**Top 3 Priority Areas**:
1. Payment processing module (tight coupling, no error handling)
2. User management (God object pattern)
3. Data fetching layer (N+1 queries, no caching)
## Priority 1: Critical Refactorings (Do First)
### 1. Decouple Payment Processing
**File**: `src/services/PaymentService.ts`
**Issue**: Tight coupling to Stripe, no abstraction
**Impact**: Cannot test, cannot swap providers
**Complexity**: High
**Time**: 2 hours
**Current Code**:
```typescript
class PaymentService {
async charge(amount: number) {
const stripe = new Stripe(process.env.STRIPE_KEY);
return stripe.charges.create({ amount });
}
}
```
**Refactored Code**:
```typescript
interface PaymentGateway {
charge(amount: number): Promise;
}
class StripeGateway implements PaymentGateway {
constructor(private apiKey: string) {}
async charge(amount: number): Promise {
const stripe = new Stripe(this.apiKey);
const charge = await stripe.charges.create({ amount });
return { id: charge.id, status: charge.status };
}
}
class PaymentService {
constructor(private gateway: PaymentGateway) {}
async charge(amount: number) {
return this.gateway.charge(amount);
}
}
```
**Benefits**:
- ā
Testable (mock gateway)
- ā
Swappable providers
- ā
Follows dependency injection
- ā
Better error handling
---
### 2. Break Up God Object: UserManager
**File**: `src/models/UserManager.ts`
**Issue**: 800+ lines, handles auth, permissions, notifications, billing
**Impact**: Hard to maintain, test, understand
**Complexity**: High
**Time**: 3 hours
**Refactoring Strategy**:
Split into:
- `User.ts` - Core user model
- `UserAuth.ts` - Authentication
- `UserPermissions.ts` - Authorization
- `UserNotifications.ts` - Email/SMS
- `UserBilling.ts` - Payment operations
**Files to Create**:
```
src/models/
āāā User.ts (core data model)
āāā UserAuth.ts
āāā UserPermissions.ts
āāā UserNotifications.ts
āāā UserBilling.ts
```
**Migration Steps**:
1. Create new files with extracted code
2. Update User class to compose services
3. Update all imports across codebase
4. Remove old methods from User class
5. Run tests to verify
---
### 3. Fix N+1 Query Problem
**File**: `src/api/routes/users.ts`
**Issue**: Loading posts in loop, 100+ queries for 100 users
**Impact**: Page load 8+ seconds, database overload
**Complexity**: Medium
**Time**: 1 hour
**Current**:
```typescript
const users = await User.findAll();
for (const user of users) {
user.posts = await Post.findByUserId(user.id); // N queries
}
```
**Refactored**:
```typescript
const users = await User.findAll({
include: [{ model: Post }] // Single join query
});
```
**Performance Impact**:
- Before: 101 queries, 8.2s load time
- After: 1 query, 0.3s load time
- **27x faster** š
---
## Priority 2: Major Refactorings (Important)
### 4. Extract Magic Numbers to Constants
**Files**: Multiple (15 files)
**Issue**: Hardcoded values throughout codebase
**Time**: 1 hour
**Examples to Fix**:
```typescript
// src/utils/validation.ts
if (password.length < 8) { } // ā
// src/config/constants.ts
export const PASSWORD_MIN_LENGTH = 8;
// src/utils/validation.ts
if (password.length < PASSWORD_MIN_LENGTH) { } // ā
```
**Files Affected**:
- `src/utils/validation.ts`
- `src/services/DiscountService.ts`
- `src/components/Pagination.tsx`
- ... (12 more)
---
### 5. Apply Strategy Pattern to Sorting
**File**: `src/utils/DataSorter.ts`
**Issue**: Giant if-else chain for 8 sort types
**Time**: 45 minutes
[... refactoring details ...]
---
### 6. Memoize React Components
**Files**: `src/components/UserCard.tsx`, `src/components/ProductCard.tsx` (8 components)
**Issue**: Unnecessary re-renders degrading performance
**Time**: 1 hour
[... refactoring details ...]
---
## Priority 3: Minor Refactorings (Nice to Have)
### 7. Improve Variable Naming
**Files**: 23 files
**Issue**: Unclear names (x, data, temp, result)
**Time**: 2 hours
**Examples**:
```typescript
function calc(a, b) { } // ā
function calculateOrderTotal(price, quantity) { } // ā
```
---
### 8. Add JSDoc Comments
**Files**: All service and util files
**Issue**: No documentation
**Time**: 2 hours
---
## Code Smell Detection
**Total Code Smells Found**: 47
### By Category:
- Long Methods: 8 instances
- Duplicate Code: 12 instances
- Large Classes: 3 instances
- Long Parameter List: 6 instances
- Feature Envy: 4 instances
- Data Clumps: 5 instances
- Primitive Obsession: 9 instances
### By File:
```
src/services/PaymentService.ts ā ļøā ļøā ļø (3 smells)
src/models/UserManager.ts ā ļøā ļøā ļøā ļøā ļø (5 smells)
src/utils/DataProcessor.ts ā ļøā ļø (2 smells)
```
## Design Pattern Opportunities
**Identified Patterns to Apply**:
1. **Strategy Pattern** - DataSorter, Validators (3 places)
2. **Factory Pattern** - User creation, Response builders (2 places)
3. **Observer Pattern** - Event handling, State updates (4 places)
4. **Decorator Pattern** - API middleware, Logging (2 places)
## Performance Optimizations
**Identified Opportunities**:
| File | Issue | Current | Optimized | Impact |
|------|-------|---------|-----------|--------|
| users.ts | N+1 queries | 8.2s | 0.3s | š High |
| UserCard.tsx | Re-renders | 45 FPS | 60 FPS | šµ Medium |
| DataProcessor.ts | O(n²) algorithm | 2.1s | 0.1s | š High |
| ProductList.tsx | No virtualization | 15 FPS | 60 FPS | š High |
## SOLID Principle Violations
**Single Responsibility**: 8 violations
- UserManager.ts
- OrderService.ts
- ReportGenerator.ts
**Open/Closed**: 3 violations
- PaymentProcessor.ts
- NotificationService.ts
**Liskov Substitution**: 0 violations ā
**Interface Segregation**: 2 violations
- IUserService (too many methods)
**Dependency Inversion**: 5 violations
- Most services (tight coupling)
## Testing Considerations
**Before Refactoring**:
- Create comprehensive tests for affected code
- Ensure 80%+ coverage before changes
- Set up regression test suite
**After Refactoring**:
- All tests must pass
- No reduction in coverage
- Performance tests should show improvement
## Implementation Plan
### Phase 1: Critical (Week 1)
- [ ] Decouple PaymentService (2h)
- [ ] Break up UserManager (3h)
- [ ] Fix N+1 queries (1h)
- [ ] Run full test suite
- **Total**: 6 hours
### Phase 2: Major (Week 2)
- [ ] Extract magic numbers (1h)
- [ ] Apply Strategy pattern (45m)
- [ ] Memoize components (1h)
- [ ] Apply Factory pattern (1h)
- [ ] Run performance benchmarks
- **Total**: 3.75 hours
### Phase 3: Minor (Week 3)
- [ ] Improve naming (2h)
- [ ] Add documentation (2h)
- [ ] Code review and adjustments (1h)
- **Total**: 5 hours
### Phase 4: Validation (Week 3)
- [ ] Final test suite run
- [ ] Performance comparison
- [ ] Code review
- [ ] Deploy to staging
- **Total**: 2 hours
**Grand Total**: 16.75 hours over 3 weeks
## Rollback Plan
If refactoring causes issues:
1. Git revert to pre-refactoring commit
2. Review test failures
3. Apply fixes incrementally
4. Re-run affected tests
**Backup Strategy**:
- Create feature branch: `refactor/payment-service`
- Keep main branch stable
- Merge only after all tests pass
## Success Metrics
**Before Refactoring**:
- Code Health: 68/100
- Test Coverage: 72%
- Performance: 8.2s avg page load
- Cyclomatic Complexity: 18 avg
- Maintainability Index: 62
**After Refactoring** (Target):
- Code Health: 85/100 (+17)
- Test Coverage: 85% (+13%)
- Performance: 0.8s avg page load (-91%)
- Cyclomatic Complexity: 8 avg (-56%)
- Maintainability Index: 82 (+20)
## Tools & Commands
```bash
# Run tests
npm test
# Check coverage
npm test -- --coverage
# Lint code
npm run lint
# Type check
npm run type-check
# Performance test
npm run perf-test
# Build
npm run build
```
## Notes
- All refactorings preserve existing functionality
- No breaking API changes
- Backward compatibility maintained
- Feature flags available for gradual rollout
- Can pause/resume at any phase boundary
## Questions?
Contact the team or create a GitHub issue for clarification on any refactoring.
```
## Detection Rules
**Code Smells Detected:**
- Methods > 50 lines
- Classes > 500 lines
- Functions with > 5 parameters
- Cyclomatic complexity > 10
- Duplicate code blocks > 10 lines
- Nested conditionals > 3 levels
**Anti-Patterns Detected:**
- God objects
- Circular dependencies
- Tight coupling
- Global state
- Magic strings/numbers
- Callback hell
**SOLID Violations:**
- Classes with multiple responsibilities
- Modification needed for extension
- Improper inheritance
- Fat interfaces
- Concrete dependencies
## Framework-Specific Rules
### React
- Unnecessary re-renders
- Missing memoization
- Prop drilling
- Large components (> 300 lines)
- No error boundaries
### Node.js/Express
- Callback hell
- No async/await usage
- Missing error middleware
- No request validation
- Hardcoded configuration
### Python/Django
- Fat models
- Business logic in views
- No service layer
- Raw SQL instead of ORM
- Missing migrations
### Go
- Error shadowing
- Goroutine leaks
- No context usage
- Global variables
- Missing defer for cleanup
## Best Practices Enforced
ā
**DRY** - Don't Repeat Yourself
ā
**KISS** - Keep It Simple, Stupid
ā
**YAGNI** - You Aren't Gonna Need It
ā
**SOLID** - Object-oriented design principles
ā
**Separation of Concerns**
ā
**Dependency Injection**
ā
**Single Responsibility**
ā
**Composition over Inheritance**
## Output Messages
**Plan Creation:**
```
ā
Refactoring Plan Generated!
š Location: /project-root/REFACTOR_PLANNING.md
Summary:
- 23 refactoring opportunities
- Estimated impact: High
- Estimated time: 16.75 hours
- Code health: 68/100 ā 85/100 (+17)
PLAN CREATED! š
```
**Implementation:**
```
ā
Refactoring Complete!
š Results:
- Files refactored: 28
- Code smells fixed: 47
- Performance improved: 91% faster
- Test coverage: 72% ā 85%
Before:
Code Health: 68/100
Page Load: 8.2s
Complexity: 18
After:
Code Health: 85/100
Page Load: 0.8s
Complexity: 8
ALL DONE! š
```
## Safety Guardrails
1. **Never refactor without tests** - If tests don't exist, create them first
2. **One refactoring at a time** - Apply incrementally
3. **Run tests after each change** - Catch regressions immediately
4. **Backup before major changes** - Use git branches
5. **Preserve functionality** - No behavior changes unless specified
6. **Document breaking changes** - If API changes, update docs
## When NOT to Refactor
ā **Skip refactoring if:**
- Code works and has good tests
- Refactoring adds no value
- Time constraints prevent proper testing
- Team lacks familiarity with pattern
- External dependencies prevent changes
- Risks outweigh benefits
ā
**Refactor when:**
- Adding new features to messy code
- Bug fixes reveal deeper issues
- Performance problems identified
- Team struggles to understand code
- Test coverage is low
- Technical debt is accumulating