Review Checklist

Core Questions

  • Does naming reflect domain intent?
  • Are there unnecessary cross-layer dependencies?
  • Is there a clearer data structure choice?
  • Is there over-abstraction or hidden magic?
  • Does error handling preserve context?
  • Do new features require edits in too many places?
  • Is the entry point obvious for users?
  • Are changes localized and reversible?

Example

A growing if-else chain signals scattered change points:

# main.py
import itertools
from sources import HNSource, V2Source, RedditSource

class NewsGrabber:
    def get_news(self, source: Optional[str] = None) -> Iterable[News]:
        if source is None:
            return itertools.chain(HNSource().iter_news(), V2Source().iter_news(), RedditSource().iter_news())
        if source == 'HN':
            return HNSource().iter_news()
        elif source == 'V2':
            return V2Source().iternews()
        elif source == 'Reddit':
            return RedditSource().iternews()
        else:
            raise ValueError(f"Not supported source: {source}")

Sources & References