> How do you recommend going after the monster function handle_key_press?
● Excellent question! The handle_key_press() function with 1,524 lines is a prime example of what needs systematic refactoring.
Let me analyze it and provide a concrete attack strategy.
● Update Todos
⎿ ☐ Analyze handle_key_press function structure
☐ Identify key functional groups and responsibilities
☐ Design refactoring strategy with safe incremental steps
☐ Test and validate refactored implementation
☐ Create helper function extraction plan
☐ Implement command dispatch table approach
● Strategic Attack Plan for handle_key_press() Monster Function
Based on my analysis, here's my recommended safe, incremental refactoring strategy:
🎯 Phase 1: Extract Helper Functions (Low Risk)
Start with the most duplicated code to get immediate wins:
1.1 Extract Layer Selection Logic
static void handle_layer_selection(int layer_num) {
char n[30];
xctx->rectcolor = layer_num;
my_snprintf(n, S(n), "%d", xctx->rectcolor);
tclvareval("xschem set rectcolor ", n, NULL);
/* ... reconfigure_layers_button logic ... */
}
Impact: Eliminates ~200 lines of duplication across numeric keys.
1.2 Extract Common Patterns
static int should_handle_key(int event, KeySym key, int state, int button) {
if(xctx->semaphore >= 2) return 0;
if(waves_selected(event, key, state, button)) {
waves_callback(event, mx, my, key, button, aux, state);
return 0;
}
return 1;
}
static void start_move_operation(int infix_interface) {
if(infix_interface) {
xctx->mx_double_save = xctx->mousex_snap;
xctx->my_double_save = xctx->mousey_snap;
move_objects(START,0,0,0);
} else {
xctx->ui_state |= MENUSTART;
xctx->ui_state2 = MENUSTARTMOVE;
}
}
🎯 Phase 2: Key Group Extraction (Medium Risk)
Extract logical groups one at a time, testing thoroughly:
2.1 Start with Numeric Keys (Safest)
static void handle_numeric_keys(KeySym key, int state, int rstate) {
if(rstate == 0) {
/* Logic level setting */
if(key >= '0' && key <= '9') {
handle_layer_selection((int)key - '0');
return;
}
}
else if(rstate == ControlMask) {
/* Layer selection with Ctrl */
// ... existing logic
}
}
2.2 Extract File Operations
static void handle_file_keys(KeySym key, int state, int rstate,
int infix_interface, const char *win_path) {
switch(key) {
case 's': handle_save_operation(state); break;
case 'S': handle_save_as_operation(); break;
case 'o': handle_open_operation(); break;
// ... etc
}
}
🎯 Phase 3: Command Dispatch Table (Higher Risk)
Replace the massive switch with a cleaner dispatch system:
3.1 Define Key Handler Structure
typedef struct {
KeySym key;
int modifier_mask;
void (*handler)(KeyHandlerContext *ctx);
const char *description;
} KeyHandler;
typedef struct {
int event, state, rstate, mx, my, button, aux;
int infix_interface, enable_stretch, cadence_compat;
int wire_draw_active, snap_cursor;
const char *win_path;
double c_snap;
KeySym key;
} KeyHandlerContext;
3.2 Create Handler Table
static const KeyHandler key_handlers[] = {
{'w', 0, handle_place_wire, "Place wire"},
{'W', 0, handle_place_snapped_wire, "Place snapped wire"},
{'c', 0, handle_copy, "Copy selection"},
{'m', 0, handle_move, "Move selection"},
// ... hundreds of entries
{0, 0, NULL, NULL} // terminator
};
static void handle_key_press_new(/* same params */) {
KeyHandlerContext ctx = {/* initialize from params */};
if(!should_handle_key(event, key, state, button)) return;
for(const KeyHandler *h = key_handlers; h->handler; h++) {
if(h->key == key && (state & h->modifier_mask) == h->modifier_mask) {
h->handler(&ctx);
return;
}
}
// Handle unmapped keys
handle_unmapped_key(&ctx);
}
🎯 Recommended Step-by-Step Attack Plan
Week 1-2: Quick Wins (Helper Functions)
1. ✅ Extract handle_layer_selection()
2. ✅ Extract should_handle_key()
3. ✅ Extract start_move_operation(), start_copy_operation()
4. ✅ Extract rotation/flip helpers
5. ✅ Run full regression tests
Expected Impact: ~400 lines reduced, 0% risk of breaking functionality
Week 3-4: First Group Extraction
1. ✅ Extract handle_numeric_keys() (safest group)
2. ✅ Extract handle_system_keys() (Delete, Escape, Return, etc.)
3. ✅ Thorough testing with focus on these key groups
4. ✅ Update any affected documentation
Expected Impact: ~300 lines reduced, <5% risk
Week 5-6: Core Groups
1. ✅ Extract handle_file_keys() (save, load, etc.)
2. ✅ Extract handle_navigation_keys() (zoom, pan)
3. ✅ Extract handle_drawing_keys() (wire, line, arc)
4. ✅ Intensive testing of drawing operations
Expected Impact: ~600 lines reduced, 10% risk
Week 7-8: Complex Groups
1. ✅ Extract handle_selection_keys() (copy, move, select)
2. ✅ Extract handle_transformation_keys() (rotate, flip)
3. ✅ Extract handle_net_keys() (highlighting, tracing)
4. ✅ Performance testing (these are hot paths)
Expected Impact: ~400 lines reduced, 15% risk
Week 9-10: Cleanup and Optimization
1. ✅ Consider dispatch table approach for remaining keys
2. ✅ Extract handle_special_keys() for symbols
3. ✅ Final integration testing
4. ✅ Performance benchmarks vs. original
Final Result: 1,524 lines → ~200 lines main function + ~1,300 lines in focused modules
🛡️ Risk Mitigation Strategies
Safety Measures
1. Git branching: Each phase gets its own branch
2. Regression testing: Full test suite after each extraction
3. Performance testing: Ensure no keystroke latency increase
4. User testing: Have power users test drawing operations
5. Rollback plan: Keep original function as handle_key_press_legacy() initially
Testing Strategy
1. Unit tests: Each extracted function gets unit tests
2. Integration tests: Full drawing workflows
3. Performance tests: Keystroke latency measurements
4. Edge case tests: Modifier combinations, rapid key sequences
5. Memory tests: Check for leaks in new function boundaries
🎉 Benefits of This Approach
Immediate Benefits (Phase 1)
- ✅ Eliminates massive code duplication
- ✅ Makes individual key handlers readable
- ✅ Zero functional risk
Medium-term Benefits (Phase 2)
- ✅ Function becomes navigable and maintainable
- ✅ New key handlers become trivial to add
- ✅ Unit testing becomes possible
Long-term Benefits (Phase 3)
- ✅ Self-documenting key binding system
- ✅ Runtime key remapping becomes possible
- ✅ Help system can auto-generate from handler table
- ✅ Plugin system for custom key handlers
This phased approach gives you immediate wins while building toward a professional-grade architecture with minimal risk to the
critical user interaction code.
No comments:
Post a Comment