code-hygiene
Skill from EvanLavender13/archctl
- Category: discovered
- Tags: github, community
- Version: 1.0.0
- Author: walid
- Downloads: 0
- Updated: 2025-11-22
Download the latest version (v1.0.0) from this page.
SKILL.md
# Code Hygiene Standards
Apply these standards when writing, refactoring, or reviewing code. Maintain clean, maintainable code that communicates intent clearly.
## Core Principles
**Comments explain WHY, not WHAT**
- Code shows what it does through clear naming and structure.
- Comments explain non-obvious design decisions, constraints, or rationale.
- Delete obvious comments that restate code logic.
**Self-documenting code > excessive comments**
- Use descriptive function names, variable names, and types.
- Extract complex expressions into well-named variables.
- Break long functions into smaller, focused functions with clear names.
**Git history captures archaeology**
- Never add "moved from X" or "extracted from Y" comments.
- Delete "refactored on DATE" or "changed by PERSON" comments.
- Git blame and git log already provide this information.
## Comment Anti-patterns
**NEVER write these comments:**
❌ **Archaeological comments** - Explain history instead of behavior
```python
# Extracted from GenerateOrchestrator._generate_code to reduce complexity
def build_context():
...
```
Why bad: Future readers don't care where it came from. Explain what it does.
❌ **Moved/refactored comments** - Document code motion
```python
# Moved from orchestrator.py on 2025-11-16
# Refactored to separate concerns
def validate_input():
...
```
Why bad: Git history shows this. Comment adds no technical value.
❌ **Self-congratulatory comments** - Pat yourself on back
```python
# This clever optimization improves performance
# Now with better error handling!
```
Why bad: Focuses on the author, not the code behavior.
❌ **Obvious comments** - Restate what code clearly shows
```python
# Increment counter
counter += 1
# Return the result
return result
```
Why bad: Code is self-explanatory. Comment adds noise.
❌ **Vague TODO comments** - No context or ownership
```python
# TODO: fix this
# TODO: refactor later
```
Why bad: No context on what needs fixing or why. Include ticket number or specific action.
❌ **Commented-out code** - Dead code left in repository
```python
# old_function()
# return legacy_value
new_function()
```
Why bad: Git remembers deleted code. Commented code rots and confuses readers.
**DO write these comments:**
✅ **Design rationale** - Explain non-obvious decisions
```python
# Use binary search here because dataset size exceeds 10k items (linear search <100ms threshold)
def find_item(dataset, target):
...
```
✅ **Security validations** - Mark critical checks with SECURITY: prefix
```python
# SECURITY: Validate path stays within project boundary to prevent traversal attacks
validate_path_within_boundary(path, root)
```
✅ **Performance constraints** - Document timing requirements
```python
# Must complete within 50ms to meet real-time physics tick rate (60Hz)
def update_physics():
...
```
✅ **External constraints** - Explain API quirks or limitations
```python
# GitHub API returns max 100 items per page; pagination required for larger datasets
results = fetch_all_pages(repo_url)
```
✅ **Actionable TODOs** - Include ticket number and specific action
```python
# TODO(#423): Replace linear search with spatial hash grid for >1000 entities
def find_nearby_entities():
...
```
## Code Quality Gates
Check these conditions before implementing changes. If ANY fail, STOP and inform the user.
### File Size Limits
**Any file exceeds 500 lines:**
- Stop and tell user: "This file exceeds 500 lines. Refactor before adding code."
- Suggest extraction targets based on responsibilities.
**Any file exceeds 300 lines and receives new code:**
- Stop and tell user: "This file already contains 300+ lines. Extract logic first?"
- Identify natural boundaries for splitting.
### Responsibility Analysis
**Before adding code to any file/class/function:**
1. Identify what responsibilities it currently owns.
2. Check if new code adds a different responsibility.
3. If yes, STOP and inform user of violation.
4. Suggest where logic should live based on Single Responsibility Principle.
5. Wait for user decision before proceeding.
### Proactive Code Smell Detection
**Push back when you detect code smells:**
- Users may not recognize accumulated technical debt.
- Call out violations BEFORE implementing changes.
- Provide specific examples of current responsibilities.
- Suggest refactoring approach.
**Examples of appropriate pushback:**
✅ "The orchestrator is 700 lines and handles discovery, loading, validation, and generation. Adding file caching will introduce a fifth responsibility. Should we extract these into separate modules first?"
✅ "This function already validates input, transforms data, and writes output (three responsibilities). Where should the logging logic actually live?"
✅ "This class is a god object managing state, I/O, validation, and business logic. Let's split it before adding more."
❌ "Okay, I'll add that feature." (when file is already bloated)
## Refactoring Hygiene
**After moving/extracting code:**
- Delete all "extracted from X" comments.
- Delete all "moved from Y to Z" comments.
- Write docstrings that explain current purpose, not history.
- Update imports to reflect new locations.
- Verify all tests still pass.
**After renaming:**
- Update all call sites atomically (avoid partial renames).
- Update documentation that references old names.
- Delete comments referencing old names.
**After deleting code:**
- Delete the code completely (don't comment it out).
- Remove imports that are no longer used.
- Remove tests for deleted functionality.
- Update documentation to reflect removal.
## Naming Standards
**Functions and variables:**
- Use descriptive names that explain purpose: `validate_path_within_boundary` not `check_path`
- Avoid abbreviations unless universally known: `HTTP`, `URL`, `ID` are fine; `ctx`, `mgr` are not
- Boolean variables start with `is_`, `has_`, `can_`: `is_valid`, `has_permission`
**Avoid vague names:**
- ❌ `data`, `info`, `temp`, `obj`, `mgr`, `handler`, `processor`
- ✅ `user_credentials`, `validation_result`, `http_response`, `contract_mapping`
**Constants:**
- Use UPPER_SNAKE_CASE: `MAX_RETRIES`, `DEFAULT_TIMEOUT_MS`
- Include units in name when applicable: `TIMEOUT_MS` not `TIMEOUT`
## Dead Code Elimination
**Never leave commented-out code:**
```python
# def old_implementation():
# ...
```
Delete it. Git remembers.
**Remove unused imports:**
```python
from typing import Dict # ← Delete if unused
```
Linters detect these. Fix immediately.
**Delete obsolete TODOs:**
- If TODO is done, delete the comment entirely.
- If TODO is obsolete, delete the comment entirely.
- If TODO is still relevant, add ticket number and context.
## Documentation Standards
**Docstrings explain behavior, not implementation:**
❌ **Implementation-focused**
```python
def process_data(items):
"""
Loops through items and calls validate() on each one.
Uses a list comprehension for efficiency.
"""
```
✅ **Behavior-focused**
```python
def process_data(items):
"""
Validate each item and return list of validation results.
Args:
items: List of data items to validate
Returns:
List of ValidationResult objects (same order as input)
Raises:
ValueError: If items list is empty
"""
```
## Quality Checklist
Before submitting code, verify:
- [ ] No "extracted from" or "moved from" comments
- [ ] No commented-out code
- [ ] No obvious comments that restate code
- [ ] No TODOs without ticket numbers or context
- [ ] No vague variable names (`data`, `temp`, `mgr`)
- [ ] No files exceed 500 lines
- [ ] No functions exceed 40 lines without justification
- [ ] All imports are used
- [ ] All functions have single, clear responsibility
- [ ] Docstrings explain behavior, not implementation
## When to Break Rules
**Long functions acceptable when:**
- Function is pure data transformation with no branching (e.g., template building)
- Function is sequential I/O pipeline with clear steps
- Breaking it up would obscure the linear flow
**Large files acceptable when:**
- File contains many small, related functions (e.g., pure utility functions)
- File is auto-generated
- File is a comprehensive test suite
**Always justify exceptions in code review.**
## Remember
Clean code communicates intent clearly. Every line should serve a purpose. Delete ruthlessly. Refactor proactively. Comment sparingly. Name precisely.