Software Design Review

// TACTICAL (bad): Quick fix that leaks implementation
const [pods, setPods] = useState<Pod[]>([]);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<Error | null>(null);
// Every component repeats this pattern...

// STRATEGIC (good): Invest in a proper abstraction
const { data: pods, isLoading, error } = useKubeQuery('pods', namespace);

// SHALLOW (bad): Many props, little functionality
interface PodCardProps {
  name: string;
  namespace: string;
  status: string;
  onSelect: () => void;
  onDelete: () => void;
  onRestart: () => void;
  onViewLogs: () => void;
  isSelected: boolean;
  showActions: boolean;
  // ...10 more props
}

// DEEP (good): Simple interface, complex behavior inside
interface PodCardProps {
  pod: Pod;
  onAction?: (action: PodAction) => void;
}

// SHALLOW (bad): Exposes all internal details
pub fn get_pods(
    client: &Client,
    namespace: &str,
    label_selector: Option<&str>,
    field_selector: Option<&str>,
    limit: Option<u32>,
    continue_token: Option<&str>,
) -> Result<Vec<Pod>>

// DEEP (good): Query object hides complexity
pub fn get_pods(query: PodQuery) -> Result<PodList>

// SPECIALIZED (bad): One method per UI action
class PodService {
  deletePodFromContextMenu(pod: Pod) { ... }
  deletePodFromKeyboard(pod: Pod) { ... }
  deletePodFromBulkAction(pods: Pod[]) { ... }
}

// GENERAL (good): One general method, UI decides how to call it
class PodService {
  deletePods(pods: Pod[]): Promise<DeleteResult> { ... }
}

// SPECIALIZED (bad): Text class knows about UI concepts
class TextEditor {
  backspace(cursor: Cursor) { ... }      // UI-specific
  deleteSelection(sel: Selection) { ... } // UI-specific
}

// GENERAL (good): Generic text operations
class TextEditor {
  delete(start: Position, end: Position) { ... }
  insert(position: Position, text: string) { ... }
}
// UI code: editor.delete(cursor.move(-1), cursor)

// SPECIALIZED (bad): API mirrors exact UI needs
pub fn get_pods_for_sidebar(namespace: &str) -> Vec<PodSummary>
pub fn get_pods_for_detail_view(name: &str) -> PodDetail
pub fn get_pods_for_search(query: &str) -> Vec<PodSearchResult>

// GENERAL (good): One flexible API
pub fn get_pods(query: PodQuery) -> Vec<Pod>
// Callers transform the data for their specific needs

// PASS-THROUGH (bad): TextDocument just forwards to TextArea
class PodManager {
  private kubeClient: KubeClient;

  getPods(namespace: string) {
    return this.kubeClient.getPods(namespace); // Just forwarding!
  }

  deletePod(namespace: string, name: string) {
    return this.kubeClient.deletePod(namespace, name); // Just forwarding!
  }
}

// BETTER: Either expose kubeClient directly or add real value
class PodManager {
  async getPodsWithMetrics(namespace: string): Promise<PodWithMetrics[]> {
    const [pods, metrics] = await Promise.all([
      this.kubeClient.getPods(namespace),
      this.metricsClient.getPodMetrics(namespace),
    ]);
    return this.mergePodMetrics(pods, metrics); // Actual value added
  }
}

// LEAK (bad): Caller must know about refresh mechanism
const { pods, refreshPods, setRefreshInterval, lastRefresh } = usePodsStore();
useEffect(() => {
  const interval = setInterval(refreshPods, refreshInterval);
  return () => clearInterval(interval);
}, [refreshInterval]);

// HIDDEN (good): Store handles refresh internally
const { pods } = usePodsStore(); // Auto-refreshes, caller doesn't care how

// LEAK (bad): Exposes kubeconfig parsing details
pub struct KubeConfig {
    pub clusters: Vec<Cluster>,
    pub contexts: Vec<Context>,
    pub current_context: String,
    pub auth_info: HashMap<String, AuthInfo>, // Internal detail!
}

// HIDDEN (good): Exposes only what callers need
impl KubeConfig {
    pub fn current_context(&self) -> &Context;
    pub fn switch_context(&mut self, name: &str) -> Result<()>;
    // Internal representation stays private
}

// COMPLEXITY PUSHED UP (bad): Every caller handles retry logic
async function fetchPods(namespace: string) {
  const response = await invoke('get_pods', { namespace });
  if (response.error) throw new Error(response.error);
  return response.data;
}
// Caller must handle: retries, timeouts, error UI, loading state...

// COMPLEXITY PULLED DOWN (good): Module handles it all
async function fetchPods(namespace: string): Promise<Pod[]> {
  return retryWithBackoff(async () => {
    const response = await invoke('get_pods', { namespace });
    return response ?? []; // Empty array if namespace doesn't exist
  }, { maxRetries: 3, timeout: 10000 });
}

// WRONG SPLIT (bad): HTTP parsing split into read + parse
// (Both need HTTP format knowledge - information leak)
async function readRequest(socket: Socket): Promise<string> { ... }
async function parseRequest(text: string): Promise<HttpRequest> { ... }

// TOGETHER (good): Single module handles both
async function readAndParseRequest(socket: Socket): Promise<HttpRequest> { ... }

// WRONG TOGETHER (bad): Generic utility mixed with UI-specific code
function useResourceList<T>() {
  const [resources, setResources] = useState<T[]>([]);
  const [selectedPodForDeletion, setSelectedPodForDeletion] = useState(null);
  // ^ UI-specific in a generic hook!
}

// SEPARATE (good): Keep generic and specific apart
function useResourceList<T>() { /* generic logic only */ }
function usePodDeletion() { /* pod-specific UI logic */ }

// MANY EXCEPTIONS (bad): Every edge case throws
function getSelectedPod(): Pod {
  if (!selectedPodId) throw new Error('No pod selected');
  const pod = pods.find(p => p.id === selectedPodId);
  if (!pod) throw new Error('Pod not found');
  return pod;
}

// DEFINED AWAY (good): Always returns valid result
function getSelectedPod(): Pod | null {
  if (!selectedPodId) return null;
  return pods.find(p => p.id === selectedPodId) ?? null;
}

// COMPLEX (bad): Caller handles all error cases
pub fn find_context(&self, name: &str) -> Result<&Context, ConfigError> {
    self.contexts.iter()
        .find(|c| c.name == name)
        .ok_or(ConfigError::ContextNotFound(name.to_string()))
}

// SIMPLE (good): Define the error away
pub fn find_context_or_current(&self, name: &str) -> &Context {
    self.contexts.iter()
        .find(|c| c.name == name)
        .unwrap_or(&self.current_context)
}

// INCONSISTENT (bad): Different patterns for same thing
const { pods } = usePodStore();        // Zustand
const [deployments] = useQuery(...);   // React Query
const services = useCustomHook();      // Custom
// ^ Three different patterns for fetching K8s resources!

// CONSISTENT (good): One pattern for K8s resources
const { data: pods } = useKubeResource('pods', namespace);
const { data: deployments } = useKubeResource('deployments', namespace);
const { data: services } = useKubeResource('services', namespace);