Loading...
Loading...
This skill analyzes code for design quality improvements across 8 dimensions: Naming, Object Calisthenics, Coupling & Cohesion, Immutability, Domain Integrity, Type System, Simplicity, and Performance. Ensures rigorous, evidence-based analysis by: (1) Understanding code flow first via implementation-analysis protocol, (2) Systematically evaluating each dimension with specific criteria, (3) Providing actionable findings with file:line references. Triggers when users request: code analysis, design review, refactoring opportunities, code quality assessment, architecture evaluation.
npx skill4agent add ntcoding/claude-skillz lightweight-design-analysislightweight-implementation-analysis-protocol❌ AVOID: getUserData(), UtilityClass, helperMethod(), DataProcessor
✅ PREFER: getUserProfile(), OrderCalculator, calculateTotal(), InvoiceGeneratorutils/helpers/common/data/doSomething()handleData()process()dataresulttempvalue🟡 Generic naming at src/utils/DataHelper.ts
- Class name "DataHelper" is too generic
- Consider: OrderValidator, CustomerRepository (based on actual responsibility)🔴 Indentation violation at User.ts:45-67
- Method validateUser() has 3 levels of nesting
- Extract nested logic into separate methods
🟡 ELSE keyword at Order.ts:23
- Can restructure with early return❌ Feature Envy:
class UserProfile {
displaySubscriptionInfo(): string {
// Accessing multiple properties of Subscription - too much interest in its data
return `Plan: ${this.subscription.planName}, ` +
`Price: $${this.subscription.monthlyPrice}/mo, ` +
`Screens: ${this.subscription.maxScreens}, ` +
`Quality: ${this.subscription.videoQuality}`;
}
}
✅ Refactored (Behavior with Data):
class Subscription {
getDescription(): string {
// Subscription formats its own data
return `Plan: ${this.planName}, ` +
`Price: $${this.monthlyPrice}/mo, ` +
`Screens: ${this.maxScreens}, ` +
`Quality: ${this.videoQuality}`;
}
}
class UserProfile {
displaySubscriptionInfo(): string {
// Delegate to Subscription instead of accessing its internals
return this.subscription.getDescription();
}
}🔴 Feature envy at OrderService.ts:34-42
- Method accesses 5 properties of Customer object
- Consider: Move logic to Customer class or extract to CustomerFormatterconstreadonly❌ AVOID:
let total = 0;
items.forEach(item => total += item.price);
✅ PREFER:
const total = items.reduce((sum, item) => sum + item.price, 0);letconstreadonlypush()pop()splice()sort()🟡 Mutable state at Cart.ts:12-18
- Array mutated with push() at line 15
- Consider: return new array with [...items, newItem]❌ Poor encapsulation / Anemic domain:
class PlaceOrderUseCase {
placeOrder(orderId) {
const order = repository.load(orderId)
if (order.getStatus() === 'DRAFT'){
order.place()
}
repository.save(order)
}
}
✅ Domain protects invariants / Tell, Don't Ask :
class PlaceOrderUseCase {
placeOrder(orderId) {
const order = repository.load(orderId)
order.place()
repository.save(order)
}
}
class Order {
...
place() {
if (this.status !== 'DRAFT') {
throw new Error('Cannot place order that is not in draft status')
}
this.status === 'PLACED'
}
}🔴 Anemic domain model at Order.ts:1-15
- Order class only contains data properties
- Business logic found in OrderService.ts:45-89
- Consider: Move calculateTotal(), validateItems() into Order classanyas❌ AVOID:
status: string; // Can be any string
✅ PREFER:
type OrderStatus = 'pending' | 'confirmed' | 'shipped' | 'delivered';
status: OrderStatus;anyasstringnumber🔴 Type safety violation at Payment.ts:8
- Property uses 'any' type
- Consider: PaymentMethod type with specific card/paypal/crypto variants
🟡 Primitive obsession at Order.ts:12
- 'status' is string, should be union type
- Consider: type OrderStatus = 'pending' | 'confirmed' | 'shipped'❌ AVOID:
function calculatePrice(item, discount, tax, shipping, insurance, gift) {
// 8 parameters handling every possible scenario
}
✅ PREFER:
function calculatePrice(item, options) {
// Simple, extensible
}🟡 Code duplication at Cart.ts:23-28 and Cart.ts:45-50
- Same validation logic duplicated
- Extract to: validateItem() method❌ AVOID:
items.forEach(item => {
const category = categories.find(c => c.id === item.categoryId); // O(n²)
});
✅ PREFER:
const categoryMap = new Map(categories.map(c => [c.id, c])); // O(n)
items.forEach(item => {
const category = categoryMap.get(item.categoryId); // O(1)
});find()filter()🔴 Performance issue at ProductList.ts:45-52
- Nested find() creates O(n²) complexity
- For 1000 items, this is 1M operations
- Use Map for O(n) solution# Design Analysis Report
**Analyzed:** [file/module name]
**Lines Reviewed:** [start-end]
## Summary
[2-3 bullet points of key findings]
---
## 🔴 Critical Issues
[Issues that should be addressed before merge/deployment]
### [Dimension] - [Brief Description]
**Location:** file.ts:line
**Issue:** [What's wrong]
**Impact:** [Why it matters]
**Recommendation:** [Specific fix]
\`\`\`typescript
// Current (problematic)
[actual code]
// Suggested
[improved code]
\`\`\`
---
## 🟡 Suggestions
[Improvements that would enhance quality]
[Same format as Critical]
---
## Metrics
- **Dimensions Evaluated:** 8/8
- **Critical Issues:** X
- **Suggestions:** Ylightweight-implementation-analysis-protocolUnderstanding UserService.ts...
- UserService.createUser() [line 23]
↓ validates user data
↓ calls database.insert() [line 45]
↓ sends email via emailService.send() [line 52]# Design Analysis Report
**Analyzed:** UserService.ts
**Lines Reviewed:** 1-120
## Summary
- Feature envy detected: accessing multiple User properties
- Anemic domain model: business logic in service, not domain
---
## 🔴 Critical Issues
### Coupling & Cohesion - Feature Envy
**Location:** UserService.ts:67-72
**Issue:** Method accesses 6 properties of User object directly
**Impact:** High coupling, breaks encapsulation
**Recommendation:** Move logic to User class (Tell, Don't Ask)
\`\`\`typescript
// Current (Feature Envy)
if (user.email && user.verified && user.role === 'admin' && user.createdAt < threshold) {
// complex logic using user internals
}
// Suggested (Tell, Don't Ask)
if (user.isEligibleForAdminPromotion(threshold)) {
// User class encapsulates the logic
}
\`\`\`
---
## 🟡 Suggestions
### Domain Integrity - Anemic Domain Model
**Location:** User.ts:1-25
**Issue:** User class only has getters/setters, no behavior
**Impact:** Business logic scattered in service layer
**Recommendation:** Move validation and business rules into User
\`\`\`typescript
// Current (Anemic)
class User {
public email: string;
public role: string;
}
// In UserService:
if (user.email && isValidEmail(user.email)) { ... }
// Suggested (Rich Domain)
class User {
private email: Email; // Value Object
validateEmail(): void {
// Invariant enforcement
}
}
\`\`\`
---
## Metrics
- **Dimensions Evaluated:** 8/8
- **Critical Issues:** 1
- **Suggestions:** 1