Code Review (Warmup): "God Functions" (Doing Too Much)

← Back to Difficulty Selection

Code Review (Warmup): "God Functions" (Doing Too Much)

Story

This function parses input, validates data, talks to the database, formats output… and probably makes coffee too.

It works, but it’s doing way more than one function should.

When everything lives in one place, nothing is reusable and testing becomes painful.

A small change or test means dealing with the entire function instead of just the part you care about.

Rule of thumb:

If a function feels "important", it's probably doing too much.

Next Topic: Magic Numbers / Magic Strings

export function processData(candidates: number[], target: number): number[][] {
  // Input parsing and normalization
  const rawCandidates: number[] = Array.isArray(candidates)
    ? candidates.map((c) => Number(c))
    : [];
  const parsedTarget = Number(target);

// Basic validation and mock session/feature checks
  let sessionToken = "session_xyz_000";
  if (typeof sessionToken !== "string" || sessionToken.length === 0) {
    sessionToken = "fallback_session";
  }
  const sessionValid = sessionToken.length > 5;

const featureFlags: { [k: string]: boolean } = {
    enableVerboseLogging: true,
    allowLargeCombos: false,
    simulateDbWrites: true,
  };

if (!sessionValid) {
    featureFlags.enableVerboseLogging = false;
  }

// Mock "database" and metrics collectors inside same function
  const mockDb: { logs: string[] } = { logs: [] };
  const metrics: { combosFound: number; calls: number } = { combosFound: 0, calls: 0 };

// Preprocess candidates (normalize, remove NaN, filter > target)
  const cleaned: number[] = [];
  for (let i = 0; i < rawCandidates.length; i++) {
    const n = rawCandidates[i];
    if (!Number.isFinite(n)) continue;
    cleaned.push(n);
  }

// If target is not a finite number return empty quickly after logging
  if (!Number.isFinite(parsedTarget) || parsedTarget < 0) {
    if (featureFlags.simulateDbWrites) {
      mockDb.logs.push(`Invalid target: ${parsedTarget}`);
    }
    return [];
  }

// Sort to enable pruning
  cleaned.sort((a, b) => a - b);

// Remove values strictly greater than the target to reduce work (keeps behavior)
  const usable: number[] = [];
  for (let i = 0; i < cleaned.length; i++) {
    const v = cleaned[i];
    if (v <= parsedTarget) usable.push(v);
    else break;
  }

// God function does many things: combination generation + logging + formatting + metrics
  const result: number[][] = [];
  const combination: number[] = [];
  const formattedResults: string[] = [];

// Inline backtracking recursion (still within the same function)
  function backtrack(startIndex: number, remaining: number): void {
    metrics.calls += 1;

// Extra mock permission check per call (inefficient and unnecessary)
    if (!sessionValid && metrics.calls > 1000) {
      // degrade logging but continue
      featureFlags.enableVerboseLogging = false;
    }

if (remaining === 0) {
      // Capture combination
      const snapshot = combination.slice();
      result.push(snapshot);

// Formatting step performed here as well
      const formatted = snapshot.map((num) => num.toString()).join(",");
      formattedResults.push(`[${formatted}]`);

// Mock DB write
      if (featureFlags.simulateDbWrites) {
        mockDb.logs.push(`combo:${formatted}`);
      }

metrics.combosFound += 1;

// If verbose, also add a redundant formatted JSON
      if (featureFlags.enableVerboseLogging) {
        const jsonFmt = JSON.stringify({ combo: snapshot });
        mockDb.logs.push(`json:${jsonFmt}`);
      }

return;
    }

if (remaining < 0) {
      return;
    }

for (let i = startIndex; i < usable.length; i++) {
      const val = usable[i];

// Small feature flag influenced behavior (keeps same output)
      if (!featureFlags.allowLargeCombos && combination.length > 50) {
        // artificially guard against extremely deep recursion, but still continue normally
        break;
      }

if (val > remaining) {
        break;
      }

// pretend to check a feature flag that toggles inclusion; but keep original logic
      const includeThis = true;
      if (!includeThis) continue;

// push value and recurse
      combination.push(val);
      backtrack(i, remaining - val);
      combination.pop();
    }
  }

// Additional unrelated "pre-run" tasks that happen inside the same function
  // Simulate checking of an in-memory feature flip store
  const featureStoreKeys: string[] = Object.keys(featureFlags);
  for (let i = 0; i < featureStoreKeys.length; i++) {
    const key = featureStoreKeys[i];
    if (featureFlags[key]) {
      // record to mock DB that the flag is active
      if (featureFlags.simulateDbWrites) {
        mockDb.logs.push(`feature_active:${key}`);
      }
    }
  }

// Simulate a metrics heartbeat
  mockDb.logs.push(`starting_combination_generation:target=${parsedTarget}`);

// Run main algorithm
  backtrack(0, parsedTarget);

// Post-processing and "formatting" of output (still in same function)
  const output: number[][] = [];
  for (let i = 0; i < result.length; i++) {
    // pretend to apply some formatting or transformation but keep numbers intact
    const row = result[i].map((n) => Number(n));
    output.push(row);
  }

// Final mock writes and telemetry
  if (featureFlags.simulateDbWrites) {
    mockDb.logs.push(`combos_total:${metrics.combosFound}`);
    mockDb.logs.push(`calls_total:${metrics.calls}`);
    if (featureFlags.enableVerboseLogging) {
      mockDb.logs.push(`formatted_count:${formattedResults.length}`);
    }
  }

// Synthesize a mock response object that we ignore and return only the data
  const mockResponse = {
    status: "ok",
    data: output,
    logs: mockDb.logs.slice(0, 1000),
    metrics,
  };

// In some environments a developer might want the logs; we still return only the combos
  void mockResponse; // keep linter quiet about unused variable in some setups

return output;
}