Apply constructor-based dependency injection with explicit configuration flow. Covers required dependencies, fail-fast principles, and avoiding optional parameters with default factories. Use when designing service constructors or managing dependencies.
Install with Codex or Claude Copy this prompt, paste it into Codex, Claude, or another assistant, and let it review the skill page and install it for you.
A direct command skips the review prompt. Inspect the source before running it.
Apply constructor-based dependency injection with explicit configuration flow. Covers required dependencies, fail-fast principles, and avoiding optional parameters with default factories. Use when designing service constructors or managing dependencies.
user-invocable
true
argument-hint
Dependency Injection
When to Use This Skill
Activate this skill when:
Designing service constructors with dependencies
Managing configuration flow from entry points to services
Deciding between required vs optional parameters
Handling missing or invalid values (fail-fast vs silent defaults)
Structuring code to avoid reading environment variables in services
"""Initialize the statistics service
Args:
repo: GitHub repository (owner/name)
metadata_service: MetadataService instance for accessing metadata
project_repository: ProjectRepository instance for loading project data
base_branch: Base branch to fetch specs from (default: "main")
"""
self
self
self
# โ Direct assignment
self
# โ Explicit dependency creation at call site
# โ Explicitly passed
Principle: Avoid Optional Dependencies with Default Factories
Optional parameters with default factory patterns (e.g., param: Optional[Type] = None with self.param = param or DefaultType()) are a code smell that hides dependencies and makes code harder to reason about.
โ Self-documenting: Code clearly shows all required collaborators
When Optional Parameters Are Acceptable
Optional parameters are acceptable only when they represent:
Truly optional behavior: Feature flags or optional enhancements
defsend_notification(message: str, priority: Optional[str] = None):
# Priority is genuinely optional - default is "normal"
Optional filters or constraints: Narrowing a query
defcollect_statistics(config_path: Optional[str] = None):
# None means "all projects", not "use default config"
Backward compatibility: When adding new parameters to existing APIs
deflegacy_method(required: str, new_param: Optional[str] = None):
# Added for backward compatibility with existing callers
Never use optional for:
Dependencies/services (use required parameters)
Configuration that should flow from the caller
Cases where None has ambiguous meaning
Principle: Fail Fast - Avoid Silent Failures
When a value is missing, invalid, or unexpected, raise an exception rather than silently returning empty data structures or default values. Missing values are often errors that should be surfaced, not hidden.
Anti-Pattern: Silent Failures (โ Avoid)
# โ BAD: Silently returning empty data when something is wrongdefget_user_config(user_id: str) -> dict:
result = database.query(f"SELECT * FROM configs WHERE user_id = '{user_id}'")
ifnot result:
return {} # โ Caller won't know if user doesn't exist or has no configdefparse_task_list(content: str) -> list[Task]:
ifnot content:
return [] # โ Was the file empty, or did we fail to read it?defget_reviewer(pr_number: int) -> Optional[str]:
pr = api.get_pull_request(pr_number)
if pr isNone:
returnNone# โ Did PR not exist, or was there an API error?return pr.get("reviewer") # โ Returns None if key missing - is that valid?# โ BAD: Using .get() with defaults to mask missing required fieldsdefprocess_config(config: dict) -> Settings:
return Settings(
timeout=config.get("timeout", 30), # โ Is 30 the right default, or should timeout be required?
retries=config.get("retries", 3), # โ Masks missing configuration
api_key=config.get("api_key", ""), # โ Empty string will fail later with confusing error
)
Problems with silent failures:
Hidden bugs: Caller proceeds with empty/default data, causing failures elsewhere
Difficult debugging: Error surfaces far from the root cause
Ambiguous meaning: Can't distinguish "no data" from "error fetching data"
False confidence: Code appears to work but produces incorrect results
Lost context: By the time the error manifests, the original cause is unknown
Recommended Pattern: Fail Fast (โ Use This)
# โ GOOD: Raise exceptions for abnormal casesdefget_user_config(user_id: str) -> dict:
result = database.query(f"SELECT * FROM configs WHERE user_id = '{user_id}'")
ifnot result:
raise UserNotFoundError(f"No configuration found for user: {user_id}")
return result
defparse_task_list(content: str) -> list[Task]:
ifnot content:
raise ValueError("Cannot parse empty content - file may be missing or unreadable")
# Parse and return tasks...defget_reviewer(pr_number: int) -> str:
pr = api.get_pull_request(pr_number)
if pr isNone:
raise PullRequestNotFoundError(f"PR #{pr_number} not found")
if"reviewer"notin pr:
raise InvalidPRDataError(f"PR #{pr_number} has no reviewer assigned")
return pr["reviewer"]
# โ GOOD: Validate required fields explicitlydefprocess_config(config: dict) -> Settings:
required_fields = ["timeout", "retries", "api_key"]
missing = [f for f in required_fields if f notin config]
if missing:
raise ConfigurationError(f"Missing required config fields: {missing}")
ifnot config["api_key"]:
raise ConfigurationError("api_key cannot be empty")
return Settings(
timeout=config["timeout"],
retries=config["retries"],
api_key=config["api_key"],
)
When Empty/Default Values Are Acceptable
Empty or default values are appropriate only when they represent valid business states, not error conditions:
# โ OK: Empty list means "no items match filter" (valid state)deffind_open_tasks(tasks: list[Task]) -> list[Task]:
return [t for t in tasks ifnot t.is_completed]
# โ OK: Optional field that genuinely may not existdefget_pr_description(pr: PullRequest) -> Optional[str]:
return pr.description # None means "no description provided" - valid state# โ OK: Default for truly optional behaviordefformat_output(data: dict, include_timestamps: bool = False) -> str:
# include_timestamps is optional enhancement, False is sensible default
...
Distinguishing Valid Empty States from Errors
Ask yourself: "Would an empty result surprise the caller and cause problems downstream?"
Scenario
Empty/Default OK?
Recommended Approach
Query returns no matching records
โ Yes
Return empty list
Required config file is missing
โ No
Raise FileNotFoundError
API call fails
โ No
Raise exception with details
Optional field not provided
โ Yes
Return None or default
Required field missing from response
โ No
Raise ValueError
User has no assigned tasks
โ Yes
Return empty list
User account doesn't exist
โ No
Raise UserNotFoundError
Benefits of Fail-Fast
โ Immediate feedback: Errors caught at the source, not downstream
โ Clear error messages: Exception describes exactly what went wrong
โ Easier debugging: Stack trace points to the actual problem
โ Explicit contracts: Function signature and behavior are unambiguous
โ No hidden state: Caller always knows if operation succeeded
โ Prevents data corruption: Invalid states don't propagate through the system
Let Exceptions Propagate in Services
Service methods should not catch exceptions and continue with default values. Let exceptions propagate to fail the calling workflow:
# โ BAD: Catch and continue with empty datadefcollect_stats(self, project: str) -> Stats:
try:
prs = self.pr_service.get_open_prs(project)
except Exception as e:
print(f"Error: {e}")
prs = [] # โ Silent failure - stats will be wrongreturn Stats(prs=prs)
# โ GOOD: Let exceptions propagatedefcollect_stats(self, project: str) -> Stats:
prs = self.pr_service.get_open_prs(project) # โ Fails if API errorsreturn Stats(prs=prs)
If the GitHub API fails, the workflow should fail - not continue with incomplete data.
Custom Exception Classes
Define specific exceptions to make error handling clear:
# domain/exceptions.pyclassClaudeChainError(Exception):
"""Base exception for ClaudeChain errors."""passclassConfigurationError(ClaudeChainError):
"""Raised when configuration is invalid or missing."""passclassProjectNotFoundError(ClaudeChainError):
"""Raised when a project doesn't exist."""passclassTaskNotFoundError(ClaudeChainError):
"""Raised when a task doesn't exist."""pass# Usagedefget_project(name: str) -> Project:
project = self.repository.find_by_name(name)
if project isNone:
raise ProjectNotFoundError(f"Project '{name}' not found")
return project
Fail-Fast Checklist
When writing code that handles potentially missing data:
Would an empty result indicate an error or a valid state?
Will the caller be able to proceed meaningfully with empty/default data?
Could this silent failure cause problems downstream?
Is the default value truly sensible, or just convenient?
Would I want to know immediately if this value was missing?
When in doubt, raise an exception. It's easier to catch and handle an exception than to debug silent failures.
Principle: Configuration Should Flow Downward
Configuration values should flow explicitly from the entry point (CLI, web handler) down through the layers. Defaults in the middle of the call stack are dangerous because callers may unintentionally rely on them, leading to bugs.
Anti-Pattern: Default Configuration Deep in Call Stack (โ Avoid)
# โ BAD: Default configuration deep in the call stackclassStatisticsService:
def__init__(self, repo: str, metadata_service: MetadataService):
self.repo = repo
self.metadata_service = metadata_service
defcollect_all_statistics(
self,
config_path: Optional[str] = None,
days_back: int = 30, # โ Default here
label: str = DEFAULT_PR_LABEL, # โ Default here
base_branch: str = "main"# โ Default here):
# Uses defaults if caller doesn't specify
...
Problems:
Silent failures: Caller forgets to pass base_branch, gets unexpected "main" instead of their intended branch
Configuration drift: Different call sites may assume different defaults
Hard to trace: Where did this value come from? Explicit parameter or default?
Testing issues: Tests pass with defaults, production fails with actual values
Unclear intent: Did caller want "main" or did they forget to specify?
Recommended Pattern: Explicit Configuration Flow (โ Use This)
# โ GOOD: Configuration flows from constructor or is required per-callclassStatisticsService:
def__init__(
self,
repo: str,
metadata_service: MetadataService,
project_repository: ProjectRepository,
base_branch: str# โ Required in constructor - set once for service lifetime):
"""All configuration required at construction"""self.repo = repo
self.metadata_service = metadata_service
self.project_repository = project_repository
self.base_branch = base_branch # โ Stored, used by all methodsdefcollect_all_statistics(
self,
config_path: Optional[str], # โ Truly optional - None has meaning
days_back: int, # โ Required - caller must decide
label: str# โ Required - caller must decide) -> StatisticsReport:
"""No defaults - uses instance variables or requires parameters"""# โ Uses instance variable set in constructor
base_branch = self.base_branch
# โ All parameters were required, no ambiguityreturnself._collect_statistics(config_path, days_back, label, base_branch)
Configuration flows from the top:
# In __main__.py - Entry point sets defaults ONCEdefmain():
args = parse_args()
# โ Defaults only at entry point, explicit about source
repo = args.repo or os.environ.get("GITHUB_REPOSITORY", "")
base_branch = args.base_branch or os.environ.get("BASE_BRANCH", "main")
days_back = args.days_back orint(os.environ.get("STATS_DAYS_BACK", "30"))
label = args.label or os.environ.get("PR_LABEL", "claudechain")
# โ Pass everything explicitly down the stackreturn cmd_statistics(
gh=gh,
repo=repo,
base_branch=base_branch,
days_back=days_back,
label=label
)
# In commands/statistics.py - Receives explicit valuesdefcmd_statistics(
gh: GitHubActionsHelper,
repo: str,
base_branch: str, # โ Required - no default
days_back: int, # โ Required - no default
label: str# โ Required - no default) -> int:
"""All configuration passed explicitly"""# โ Create service with configuration
service = StatisticsService(repo, metadata_service, project_repo, base_branch)
# โ Call with explicit parameters
report = service.collect_all_statistics(
config_path=None,
days_back=days_back,
label=label
)
Benefits of Explicit Configuration Flow
โ Single source of truth: Defaults defined once at entry point
โ Traceable: Easy to see where values come from (CLI arg, env var, or hardcoded default)
โ Intentional: Every caller must consciously provide or accept values
โ Fail fast: Missing configuration caught at entry point, not deep in call stack
โ Testable: Tests must explicitly specify values, catching assumptions
โ No surprises: What caller passes is what service uses - no hidden defaults
When Defaults Are Acceptable
Use defaults only in these specific cases:
Entry point only: CLI argument parsers, main() functions
# โ OK: Default at entry point
parser.add_argument("--days-back", type=int, default=30)
True behavioral flags: Optional features that are genuinely off/on
# โ OK: Feature flag with clear defaultdefformat_output(data: dict, include_debug: bool = False):
# Debug output is optional, False is the natural default
Backward compatibility: When extending existing APIs
# โ OK: New parameter defaults to old behaviordeflegacy_api(required: str, new_feature: bool = False):
# False preserves existing behavior
Business logic parameters (user IDs, project names, dates)
Any value where the caller should make a conscious choice
Configuration Checklist
When adding a new parameter, ask:
Should this be in the constructor (applies to all operations)?
Should this be a method parameter (varies per call)?
Does this need a default, or should it be required?
If it has a default, is it truly optional or just convenient?
Will callers understand what happens if they omit this?
Can this lead to silent failures if the wrong default is used?
Default to making parameters required - only add defaults with clear justification.
Principle: Services Should Not Read Environment Variables
Service classes and their methods should never read environment variables directly using os.environ.get(). All environment variable access should happen at the entry point layer (CLI commands, web handlers, etc.) and be passed explicitly as constructor or method parameters.
