《Clean Code》实践疑问：双参数函数是否为不良设计？

Great question—this is exactly the kind of practical tradeoff between readability, performance, and Clean Code ideals that developers grapple with every day. Let’s break this down step by step.

First: Is Your Current Two-Parameter Approach Acceptable? #

Short answer: It’s acceptable, but with caveats.

Clean Code emphasizes minimizing function parameters (0 > 1 > 2 > N) as a guideline to reduce cognitive load and improve clarity, but this isn’t a rigid rule. In your case, you’re making a deliberate tradeoff: accepting two parameters to avoid the performance hit of searching large arrays for an error instance.

That said, there are two key risks to watch for:

• Ambiguity: A caller could pass an index that doesn’t match the error’s actual position in its target array (e.g., using a SignatureErrors index for an ARSyncError), leading to silent failures or incorrect deletions.
• Weak type safety: Your current function signature doesn’t enforce that the index corresponds to the error type, which can introduce bugs in TypeScript if not carefully managed.

If your error arrays are small (so search performance isn’t a real concern) or callers frequently mix up indexes and error types, this approach might cause more problems than it solves. But if performance is a legitimate bottleneck (e.g., arrays with thousands of errors, frequent deletion operations), this tradeoff is reasonable.

Optimizations to Improve the Implementation #

If you want to align closer with Clean Code principles while preserving (or enhancing) performance, here are actionable tweaks:

1. Wrap Parameters in a Descriptive Object #

Instead of passing two separate parameters, encapsulate them in an object. This makes the function’s intent clearer and avoids parameter order confusion:

deleteError(options: { error: DocSyncError; index: number }) {
  const { error, index } = options;
  if (error instanceof SignatureSyncError) {
    this.SignatureErrors.splice(index, 1);
  } else if (error instanceof ARSyncError) {
    this.ArErrors.splice(index, 1);
  } else if (error instanceof EnrSyncError) {
    this.EnrSyncError.splice(index, 1);
  }
}

Callers now write deleteError({ error: mySignatureError, index: 3 }), which is far more readable than positional parameters.

2. Add Type Overloads for Safety #

In TypeScript, use function overloads to enforce that the index is paired with the correct error type. This eliminates the risk of mismatched parameters:

// Overloads define valid type pairs
deleteError(error: SignatureSyncError, index: number): void;
deleteError(error: ARSyncError, index: number): void;
deleteError(error: EnrSyncError, index: number): void;

// Implementation with union type
deleteError(error: DocSyncError, index: number) {
  if (error instanceof SignatureSyncError) {
    this.SignatureErrors.splice(index, 1);
  } else if (error instanceof ARSyncError) {
    this.ArErrors.splice(index, 1);
  } else if (error instanceof EnrSyncError) {
    this.EnrSyncError.splice(index, 1);
  }
}

Now TypeScript will throw an error if someone tries to pass an ARSyncError with an index meant for SignatureErrors—a huge win for type safety.

3. Use Unique IDs + a Lookup Map (No Index Needed) #

To avoid passing an index entirely without sacrificing performance, add a unique id property to each DocSyncError subclass, then maintain a map tracking each error’s ID to its type and index:

// Add an id property to your base error class
abstract class DocSyncError {
  public readonly id: string = crypto.randomUUID(); // Or another unique generator
}

class SignatureSyncError extends DocSyncError { /* ... */ }
class ARSyncError extends DocSyncError { /* ... */ }
class EnrSyncError extends DocSyncError { /* ... */ }

// Update SyncErrorList to manage the lookup map
export class SyncErrorList {
  public SignatureErrors: SignatureSyncError[] = [];
  public ArErrors: ARSyncError[] = [];
  public EnrSyncError: EnrSyncError[] = [];
  
  private errorLookup = new Map<string, { type: 'signature' | 'ar' | 'enr', index: number }>();

  // Update the lookup map when adding errors
  addSignatureError(error: SignatureSyncError) {
    this.SignatureErrors.push(error);
    this.errorLookup.set(error.id, { type: 'signature', index: this.SignatureErrors.length - 1 });
  }

  // Similar add methods for other error types...

  // Delete only needs the error instance now
  deleteError(error: DocSyncError) {
    const entry = this.errorLookup.get(error.id);
    if (!entry) return;

    switch (entry.type) {
      case 'signature':
        this.SignatureErrors.splice(entry.index, 1);
        break;
      case 'ar':
        this.ArErrors.splice(entry.index, 1);
        break;
      case 'enr':
        this.EnrSyncError.splice(entry.index, 1);
        break;
    }
    this.errorLookup.delete(error.id);
    // Don't forget to reindex the lookup map for remaining errors after splice!
  }
}

This approach removes the need for callers to track indexes entirely, aligns with Clean Code’s preference for single-parameter functions, and keeps performance fast via the lookup map. The only extra work is maintaining the map when errors are added or removed.

4. Refactor to a Single Error Array (If Possible) #

If your business logic allows, store all errors in a single array with a type discriminator. This simplifies the delete function drastically:

type SyncError = 
  | { type: 'signature', error: SignatureSyncError }
  | { type: 'ar', error: ARSyncError }
  | { type: 'enr', error: EnrSyncError };

export class SyncErrorList {
  public errors: SyncError[] = [];

  deleteErrorByIndex(index: number) {
    this.errors.splice(index, 1);
  }

  // Or delete by error instance (with optional lookup map for performance)
  deleteError(error: DocSyncError) {
    const index = this.errors.findIndex(e => e.error === error);
    if (index !== -1) this.errors.splice(index, 1);
  }
}

This is the cleanest solution if you don’t need separate arrays for other parts of your codebase.

Final Takeaway #

Your original two-parameter approach is acceptable if performance is critical and you mitigate mismatched parameter risks (via TypeScript overloads or clear documentation). But if you can prioritize readability and type safety without significant performance loss, the optimized approaches above will make your code more aligned with Clean Code principles.

内容的提问来源于stack exchange，提问作者Amanite Laurine