refactor: Eliminate duplicate code patterns across commands #16
Conversation
Implements comprehensive DRY refactoring to consolidate repetitive code patterns across command files. This reduces code duplication by ~327 lines while improving maintainability and consistency. Key Changes: - Create command-helpers.ts with common utilities (wallet validation, chain config validation, error handling, password prompts, cancellation checks) - Create safe-helpers.ts for Safe-specific operations (Safe selection, blockchain data fetching, owner verification, address parsing, transaction selection) - Create command-context.ts for dependency injection pattern (consolidates storage initialization into single function) - Refactor 11 commands to use new helpers and context - Add deprecation notice to utils/validation.ts, preferring ValidationService Benefits: - Reduced codebase by 327 lines (214 additions, 541 deletions) - Consistent error handling and validation across all commands - Single point of initialization for command dependencies - Improved testability through helper function isolation - Better type safety with CommandContext interface Commands refactored: - account/add-owner.ts - account/remove-owner.ts - account/change-threshold.ts - tx/create.ts - tx/sign.ts - tx/execute.ts - wallet/import.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors command files to eliminate code duplication by extracting common patterns into reusable helper utilities. It consolidates wallet validation, chain configuration checks, Safe selection, password prompts, and error handling into three new helper modules (command-helpers.ts, safe-helpers.ts, and command-context.ts), reducing the codebase by 327 lines while improving consistency and maintainability.
Key Changes:
- Introduces three new utility modules for common command operations (context initialization, validation helpers, Safe-specific operations)
- Refactors 7 command files to use these new helpers
- Adds deprecation notice to existing validation utilities
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/validation.ts | Adds deprecation notice directing to ValidationService for command-level usage |
| src/utils/safe-helpers.ts | New utility module providing Safe selection, blockchain data fetching, owner verification, and transaction selection |
| src/utils/command-helpers.ts | New utility module providing wallet/chain validation, error handling, password prompts, and cancellation checks |
| src/utils/command-context.ts | New module implementing dependency injection pattern for command initialization |
| src/commands/wallet/import.ts | Refactored to use promptPassword and checkCancelled helpers |
| src/commands/tx/sign.ts | Refactored to use command context and all helper utilities |
| src/commands/tx/execute.ts | Refactored to use command context and helper utilities |
| src/commands/tx/create.ts | Refactored to use validation and cancellation helpers |
| src/commands/account/remove-owner.ts | Refactored to use command context and Safe-specific helpers |
| src/commands/account/change-threshold.ts | Refactored to use command context and Safe-specific helpers |
| src/commands/account/add-owner.ts | Refactored to use command context and Safe-specific helpers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| safeStorage: SafeAccountStorage, | ||
| configStore: ConfigStore, | ||
| statusFilter?: TransactionStatus[], | ||
| message: string = 'Select transaction' |
There was a problem hiding this comment.
[nitpick] The default parameter should use a more specific message or avoid a default altogether. 'Select transaction' is generic and the function is always called with an explicit message in the refactored code. Consider removing the default value since all call sites provide explicit messages like 'Select transaction to sign' or 'Select transaction to execute'.
| message: string = 'Select transaction' | |
| message: string |
| spinner.start('Creating add owner transaction...') | ||
|
|
||
| // The addOwnerWithThreshold method encodes the transaction data | ||
| const txService = new TransactionService(chain) |
There was a problem hiding this comment.
The TransactionService is instantiated after the Safe data has already been fetched using fetchSafeOwnersAndThreshold, which also creates its own TransactionService instance. Consider either passing the TransactionService instance through the helper function or creating it once at the beginning of the command to avoid multiple instantiations.
Implements comprehensive DRY refactoring to consolidate repetitive code patterns across command files. This reduces code duplication by ~327 lines while improving maintainability and consistency.
Key Changes:
Benefits:
Commands refactored:
🤖 Generated with Claude Code