🏗️ Code Refactoring with SOLID Principles

When to use this skill

Use this skill when:

• Refactoring existing code to improve maintainability

• Code has grown complex and difficult to modify

• Adding new features requires changing multiple unrelated parts

• Unit testing is difficult due to tight coupling

• Code violates SOLID principles

• Planning architectural improvements

How to use it

🎯 SOLID Principles Overview

S - Single Responsibility Principle (SRP)

Definition: A class should have only one reason to change.

Violations in current codebase:

❌ BAD: DataManager does too much

class DataManager: def load_data(self, data_type): # Loading logic def save_data(self, data_type, data): # Saving logic def delete_data(self, data_type): # Deletion logic # Also handles Firebase connection, validation, etc.

Refactored:

✅ GOOD: Separate responsibilities

class FirebaseConnection: """Handles Firebase connection only""" def connect(self): pass def is_available(self): pass

class DataRepository: """Handles data CRUD operations only""" def init(self, connection: FirebaseConnection): self.connection = connection

def load(self, data_type): pass
def save(self, data_type, data): pass
def delete(self, data_type): pass

class DataValidator: """Handles data validation only""" def validate_group_ids(self, ids): pass def validate_schedule(self, schedule): pass

O - Open/Closed Principle (OCP)

Definition: Software entities should be open for extension but closed for modification.

Violations:

❌ BAD: Must modify function to add new command

def handle_message(event): if event.text.startswith("@time"): # handle time elif event.text.startswith("@day"): # handle day elif event.text.startswith("@week"): # handle week # Adding new command requires modifying this function

Refactored:

✅ GOOD: Command pattern - add new commands without modifying handler

class Command(ABC): @abstractmethod def can_handle(self, text: str) -> bool: pass

@abstractmethod
def execute(self, event) -> str:
    pass

class TimeCommand(Command): def can_handle(self, text: str) -> bool: return text.startswith("@time")

def execute(self, event) -> str:
    # Handle time command
    pass

class CommandHandler: def init(self): self.commands: List[Command] = []

def register(self, command: Command):
    self.commands.append(command)

def handle(self, event):
    for command in self.commands:
        if command.can_handle(event.text):
            return command.execute(event)

L - Liskov Substitution Principle (LSP)

Definition: Objects of a superclass should be replaceable with objects of its subclasses.

Application:

✅ GOOD: Storage abstraction

class Storage(ABC): @abstractmethod def save(self, key: str, value: Any) -> bool: pass

@abstractmethod
def load(self, key: str) -> Any:
    pass

class FirebaseStorage(Storage): def save(self, key: str, value: Any) -> bool: # Firebase implementation pass

def load(self, key: str) -> Any:
    # Firebase implementation
    pass

class LocalFileStorage(Storage): def save(self, key: str, value: Any) -> bool: # File implementation pass

def load(self, key: str) -> Any:
    # File implementation
    pass

Can swap implementations without changing client code

def save_schedule(storage: Storage, schedule): storage.save("schedule", schedule)

I - Interface Segregation Principle (ISP)

Definition: Clients should not be forced to depend on interfaces they don't use.

Violations:

❌ BAD: Fat interface

class BotService: def handle_message(self): pass def handle_join(self): pass def handle_leave(self): pass def send_broadcast(self): pass def schedule_task(self): pass def validate_input(self): pass # Too many responsibilities

Refactored:

✅ GOOD: Segregated interfaces

class MessageHandler(ABC): @abstractmethod def handle_message(self, event): pass

class GroupEventHandler(ABC): @abstractmethod def handle_join(self, event): pass @abstractmethod def handle_leave(self, event): pass

class BroadcastService(ABC): @abstractmethod def send_broadcast(self, group_id: str, message: str): pass

class ScheduleService(ABC): @abstractmethod def schedule_task(self, task): pass

D - Dependency Inversion Principle (DIP)

Definition: High-level modules should not depend on low-level modules. Both should depend on abstractions.

Violations:

❌ BAD: Direct dependency on concrete class

class ScheduleManager: def init(self): self.firebase = firebase_service.firebase_service_instance # Tight coupling

def save_schedule(self, schedule):
    self.firebase.save_group_schedules(schedule)

Refactored:

✅ GOOD: Depend on abstraction

class ScheduleRepository(ABC): @abstractmethod def save(self, schedule): pass @abstractmethod def load(self): pass

class ScheduleManager: def init(self, repository: ScheduleRepository): self.repository = repository # Depends on abstraction

def save_schedule(self, schedule):
    self.repository.save(schedule)

Concrete implementation

class FirebaseScheduleRepository(ScheduleRepository): def init(self, firebase_service): self.firebase = firebase_service

def save(self, schedule):
    return self.firebase.save_group_schedules(schedule)

def load(self):
    return self.firebase.load_group_schedules()

🔧 Refactoring Patterns

1. Extract Class

When: A class is doing too much

Before

class BotHandler: def parse_time(self, text): pass def parse_members(self, text): pass def validate_time(self, hour, minute): pass def validate_members(self, members): pass def format_message(self, action, details): pass def handle_command(self, event): pass

After

class InputParser: def parse_time(self, text): pass def parse_members(self, text): pass

class InputValidator: def validate_time(self, hour, minute): pass def validate_members(self, members): pass

class MessageFormatter: def format_success(self, action, details): pass def format_error(self, error): pass

class BotHandler: def init(self, parser, validator, formatter): self.parser = parser self.validator = validator self.formatter = formatter

def handle_command(self, event): pass

2. Extract Method

When: A method is too long or does multiple things

Before

def handle_time_command(event): parts = event.text.split() if len(parts) < 2: return "Error" time_str = parts[1] patterns = [r'^(\d{1,2}):(\d{2})$', r'^(\d{2})(\d{2})$'] hour, minute = None, None for pattern in patterns: match = re.match(pattern, time_str) if match: hour = int(match.group(1)) minute = int(match.group(2)) break if not (0 <= hour <= 23): return "Hour error" # ... more logic

After

def handle_time_command(event): time_str = extract_time_parameter(event.text) if not time_str: return format_missing_parameter_error()

hour, minute, error = parse_time_flexible(time_str)
if error:
    return error

return update_schedule_time(event.group_id, hour, minute)

def extract_time_parameter(text: str) -> Optional[str]: parts = text.split(maxsplit=1) return parts[1] if len(parts) >= 2 else None

3. Introduce Parameter Object

When: Functions have too many parameters

Before

def update_schedule(group_id, days, hour, minute, timezone, enabled): pass

After

@dataclass class ScheduleConfig: group_id: str days: str hour: int minute: int timezone: str = "Asia/Taipei" enabled: bool = True

def update_schedule(config: ScheduleConfig): pass

4. Replace Conditional with Polymorphism

When: Complex if/elif chains for different types

Before

def format_message(message_type, data): if message_type == "success": return f"✅ {data['action']}\n{data['details']}" elif message_type == "error": return f"❌ {data['error']}\n{data['suggestion']}" elif message_type == "warning": return f"⚠️ {data['warning']}"

After

class Message(ABC): @abstractmethod def format(self) -> str: pass

class SuccessMessage(Message): def init(self, action: str, details: dict): self.action = action self.details = details

def format(self) -> str:
    return f"✅ {self.action}\n{self.details}"

class ErrorMessage(Message): def init(self, error: str, suggestion: str): self.error = error self.suggestion = suggestion

def format(self) -> str:
    return f"❌ {self.error}\n{self.suggestion}"

📋 Refactoring Checklist

Before Refactoring

• Write tests for existing functionality

• Identify code smells (long methods, large classes, duplicate code)

• Document current behavior

• Create backup/branch

During Refactoring

• Make small, incremental changes

• Run tests after each change

• Keep commits atomic and descriptive

• Maintain backward compatibility if needed

After Refactoring

• Verify all tests pass

• Check performance hasn't degraded

• Update documentation

• Code review

🚨 Code Smells to Watch For

• Long Method (>20 lines) → Extract Method

• Large Class (>200 lines) → Extract Class

• Long Parameter List (>3 params) → Introduce Parameter Object

• Duplicate Code → Extract Method/Class

• Feature Envy (method uses another class more than its own) → Move Method

• Data Clumps (same group of data together) → Extract Class

• Primitive Obsession (using primitives instead of objects) → Introduce Value Object

• Switch Statements (type checking) → Replace with Polymorphism

🎯 Refactoring Priority for Current Codebase

High Priority

• Extract Command Handlers - Massive handle_message() function

• Separate Data Access - DataManager does too much

• Introduce Storage Abstraction - Tight coupling to Firebase

Medium Priority

• Extract Parsing Logic - Scattered parsing code

• Introduce Message Formatters - Duplicate formatting code

• Extract Validation - Validation mixed with business logic

Low Priority

• Introduce Value Objects - For ScheduleConfig, MemberGroup

• Extract Helper Functions - Utility functions in main file

📚 References

• Clean Code by Robert C. Martin

• Refactoring by Martin Fowler

• Design Patterns by Gang of Four

• Python SOLID Principles

• Refactoring Guru

Last Updated: 2026-01-16

Maintainer: Code Quality Agent