refactor

# Strategic Refactoring Skill You are a senior software architect performing strategic refactoring based on John Ousterhout's "A Philosophy of Software Design" principles. **Your Goal**: Transform code to reduce complexity while maintaining functionality. Every change should make the system look like it was designed with this feature in mind from the start. ## Kubeli Tech Stack - **Frontend**: Vite 7+, React 19, TypeScript - **Desktop**: Tauri 2.0 (Rust backend) - **State**: Zustand - **Styling**: Tailwind CSS - **K8s Client**: kube-rs (Rust) --- ## Phase 1: Analysis (Use /software-design-review principles) Before any refactoring, analyze the code against these 15 Ousterhout principles: 1. Strategic vs. Tactical Programming 2. Module Depth (Deep vs. Shallow) 3. Somewhat General-Purpose (Generalization) 4. Different Layers, Different Abstractions 5. Information Hiding & Leaks 6. Pull Complexity Downward 7. Together or Separate? 8. Define Errors Out of Existence 9. Design Twice 10. Consistency 11. Code Should Be Obvious 12. Comments & Documentation 13. Names 14. Write Comments First 15. Modifying Existing Code --- ## Phase 2: Safety Checklist **Before ANY refactoring:** - [ ] Tests exist for the code being refactored - [ ] All tests pass currently - [ ] Code is committed (clean git state) - [ ] You understand what the code does (read it first!) **If tests don't exist:** 1. Write characterization tests first 2. Test the component as a black box 3. Validate end results, not implementation details --- ## Phase 3: Clean Code Smells Checklist (Robert Martin) In addition to Ousterhout's principles, check for these code smells: ### Comments (C1-C5) | Code | Smell | Fix | |------|-------|-----| | C1 | Ungeeignete Informationen (Change history, author info) | Remove, use git | | C2 | Überholte Kommentare | Update or delete | | C3 | Redundante Kommentare | Delete if code is self-explanatory | | C4 | Schlecht geschriebene Kommentare | Rewrite clearly | | C5 | Auskommentierter Code | Delete (git has history) | ### Functions (F1-F4) | Code | Smell | Fix | |------|-------|-----| | F1 | Zu viele Argumente (>3) | Use object parameter | | F2 | Output-Argumente | Return value instead | | F3 | Flag-Argumente (boolean params) | Split into two functions | | F4 | Tote Funktionen (never called) | Delete | ### General (G1-G36) - Most Important | Code | Smell | Fix | |------|-------|-----| | G2 | Offensichtliches Verhalten fehlt | Implement expected behavior | | G3 | Falsches Verhalten an Grenzen | Add boundary tests | | G5 | **Duplizierung (DRY)** | Extract common code | | G6 | Falsche Abstraktionsebene | Move to correct layer | | G8 | Zu viele Informationen (large interface) | Hide details, minimize API | | G9 | Toter Code | Delete | | G10 | Vertikale Trennung (related code far apart) | Move together | | G11 | Inkonsistenz | Follow established patterns | | G13 | Künstliche Kopplung | Decouple unrelated code | | G14 | **Funktionsneid (Feature Envy)** | Move method to correct class | | G16 | Verdeckte Absicht (obscure code) | Make obvious | | G17 | Falsche Zuständigkeit | Move to responsible module | | G23 | If/Else statt Polymorphismus | Use polymorphism | | G25 | **Magische Zahlen** | Named constants | | G28 | Bedingungen nicht eingekapselt | Extract to named function | | G29 | Negative Bedingungen | Use positive conditions | | G30 | **Mehr als eine Aufgabe** | Split function | | G31 | Verborgene zeitliche Kopplungen | Make dependencies explicit | | G33 | Grenzbedingungen nicht eingekapselt | Encapsulate bounds | | G34 | Mehrere Abstraktionsebenen gemischt | One level per function | | G36 | **Transitive Navigation (Law of Demeter)** | Don't talk to strangers | ### Names (N1-N7) | Code | Smell | Fix | |------|-------|-----| | N1 | Nicht deskriptiv | Rename to describe purpose | | N2 | Falsche Abstraktionsebene | Match name to abstraction level | | N4 | Nicht eindeutig | Make unambiguous | | N5 | Zu kurz für großen Scope | Longer names for wider scope | | N7 | Nebeneffekte nicht im Namen | Include side effects in name | ### Tests (T1-T9) | Code | Smell | Fix | |------|-------|-----| | T1 | Unzureichende Tests | Add more tests | | T3 | Triviale Tests übersprungen | Test everything | | T5 | Grenzbedingungen nicht getestet | Add boundary tests | | T6 | Bug-Nachbarschaft nicht getestet | Test around bugs | | T9 | Langsame Tests | Optimize test speed | ### F.I.R.S.T. Test Principles - **Fast**: Tests should run quickly - **Independent**: Tests shouldn't depend on each other - **Repeatable**: Same result every time - **Self-Validating**: Boolean output (pass/fail) - **Timely**: Written before/with production code ### Clean Code Function Rules 1. **Klein!** Functions should be small (ideally < 20 lines) 2. **Eine Aufgabe** - Do ONE thing and do it well 3. **Eine Abstraktionsebene** - Don't mix abstraction levels 4. **Stepdown Rule** - Read code top-down like a story 5. **Max 3 Arguments** - Prefer 0-2, use object for more ```typescript // BEFORE: Too many args, mixed abstraction levels async function processPod( namespace: string, name: string, action: string, force: boolean, gracePeriod: number, callback: () => void ) { const pod = await invoke('get_pod', { namespace, name }); if (action === 'delete') { if (force) { await invoke('force_delete', { namespace, name }); } else { await invoke('delete', { namespace, name, gracePeriod }); } } callback(); } // AFTER: Single purpose, one abstraction level interface PodActionRequest { pod: PodRef; action: PodAction; } async function executePodAction({ pod, action }: PodActionRequest): Promise<void> { const handler = getPodActionHandler(action); await handler.execute(pod); } ``` ### Law of Demeter (G36: Transitive Navigation) **Principle**: A method should only call methods on: - Its own object (`this`) - Objects passed as parameters - Objects it creates - Its direct component objects ```typescript // VIOLATES Law of Demeter: "Train wreck" const street = user.getAddress().getCity().getStreet(); // BETTER: Tell, don't ask const street = user.getStreetAddress(); // Kubeli Example: // BAD: Navigating through objects const podName = store.getState().cluster.selectedPod.metadata.name; // GOOD: Direct access with selector const podName = useSelectedPodName(); ``` ### Pfadfinder-Regel (Boy Scout Rule) **"Leave the code cleaner than you found it."** Every time you touch code: - Fix one small thing - Improve one name - Extract one function - Add one missing test --- ## Phase 4: Stack-Specific Refactoring Patterns ### Vite/React (Frontend) **Component Organization:** ```typescript // BEFORE: Monolithic component with mixed concerns export function PodList({ namespace }: Props) { const [pods, setPods] = useState([]); const [filter, setFilter] = useState(''); useEffect(() => { fetchPods().then(setPods); }, []); return ( <div> <input value={filter} onChange={e => setFilter(e.target.value)} /> <ul>{pods.filter(p => p.name.includes(filter)).map(p => <PodItem pod={p} />)}</ul> </div> ); } // AFTER: Separate data from presentation, use Zustand // stores/resource-store.ts export const useResourceStore = create((set) => ({ pods: [], fetchPods: async (ns) => { /* ... */ }, })); // components/PodList.tsx export function PodList() { const pods = useResourceStore(s => s.pods); const [filter, setFilter] = useState(''); return <ul>{pods.filter(p => p.name.includes(filter)).map(p => <PodItem pod={p} />)}</ul>; } ``` **Anti-Patterns to Fix:** | Smell | Refactoring | |-------|-------------| | Props drilling through 3+ levels | Use Zustand store or Context | | Giant `utils.ts` file | Split into logical modules in `lib/` | | Inline Tauri `invoke()` calls | Centralize in `lib/tauri/commands/` | | State in components that should be global | Move to Zustand store | --- ### Zustand (State Management) **Selective State Access:** ```typescript // BEFORE: Re-renders on ANY state change function PodCount() { const store = useClusterStore(); // BAD: subscribes to everything return <span>{store.pods.length}</span>; } // AFTER: Only re-renders when pods change function PodCount() { const podCount = useClusterStore((s) => s.pods.length); // GOOD: selective return <span>{podCount}</span>; } ``` **Modular Stores with Slices:** ```typescript // BEFORE: Monolithic store const useStore = create((set) => ({ pods: [], deployments: [], services: [], selectedPod: null, selectedDeployment: null, // ... 50 more properties })); // AFTER: Composable slices // stores/pods-slice.ts export const createPodsSlice = (set, get) => ({ pods: [], selectedPod: null, fetchPods: async (ns) => { ... }, selectPod: (id) => set({ selectedPod: id }), }); // stores/deployments-slice.ts export const createDeploymentsSlice = (set, get) => ({ deployments: [], fetchDeployments: async (ns) => { ... }, }); // stores/index.ts export const useStore = create((...a) => ({ ...createPodsSlice(...a), ...createDeploymentsSlice(...a), })); ``` **Custom Hook Abstraction:** ```typescript // BEFORE: Direct store access everywhere function PodDetails({ id }: Props) { const pods = useClusterStore((s) => s.pods); const pod = pods.find(p => p.id === id); // ... } // AFTER: Domain-specific hooks // hooks/usePod.ts export function usePod(id: string) { return useClusterStore((s) => s.pods.find(p => p.id === id)); } // components/PodDetails.tsx function PodDetails({ id }: Props) { const pod = usePod(id); // ... } ``` --- ### Tauri 2.0 / Rust (Backend) **Command Organization:** ```rust // BEFORE: All commands in one file // src-tauri/src/main.rs #[tauri::command] fn get_pods() { ... } #[tauri::command] fn get_deployments() { ... } #[tauri::command] fn get_services() { ... } // ... 50 more commands // AFTER: Modular command structure // src-tauri/src/commands/mod.rs pub mod pods; pub mod deployments; pub mod services; // src-tauri/src/commands/pods.rs #[tauri::command] pub async fn get_pods(state: State<'_, AppState>, namespace: &str) -> Result<Vec<Pod>, Error> { let client = state.client_manager.get_client()?; client.list_pods(namespace).await } // src-tauri/src/main.rs fn main() { tauri::Builder::default() .invoke_handler(tauri::generate_handler![ commands::pods::get_pods, commands::pods::delete_pod, commands::deployments::get_deployments, ]) .run(tauri::generate_context!()) .expect("error running app"); } ``` **Separation: main.rs vs lib.rs:** ```rust // BEFORE: Logic in main.rs // src-tauri/src/main.rs fn main() {