# 不佳
if user.age > 18:
pass
# 良好
MINIMUM_AGE = 18
if user.age > MINIMUM_AGE:
pass
深层嵌套：
# 不佳
if condition1:
if condition2:
if condition3:
if condition4:
# 深层嵌套的代码
# 良好（提前返回）
if not condition1:
return
if not condition2:
return
if not condition3:
return
if not condition4:
return
# 扁平化的代码
安全漏洞
SQL 注入：
# 不佳
query = f"SELECT * FROM users WHERE id = {user_id}"
# 良好
query = "SELECT * FROM users WHERE id = %s"
cursor.execute(query, (user_id,))
# Good
def test_user_creation_with_valid_data_succeeds():
pass
# Bad
def test1():
pass
Step 7: Documentation review
Code comments :
Complex logic explained
No obvious comments
TODOs have tickets
Comments are accurate
Function documentation :
def calculate_total(items: List[Item], tax_rate: float) -> Decimal:
"""
Calculate the total price including tax.
Args:
items: List of items to calculate total for
tax_rate: Tax rate as decimal (e.g., 0.1 for 10%)
Returns:
Total price including tax
Raises:
ValueError: If tax_rate is negative
"""
pass
README/docs :
README updated if needed
API docs updated
Migration guide if breaking changes
Step 8: Provide feedback
Be constructive :
✅ Good:
"Consider extracting this logic into a separate function for better
testability and reusability:
def validate_email(email: str) -> bool:
return '@' in email and '.' in email.split('@')[1]
This would make it easier to test and reuse across the codebase."
❌ Bad:
"This is wrong. Rewrite it."
Be specific :
✅ Good:
"On line 45, this query could cause N+1 problem. Consider using
.select_related('author') to fetch related objects in a single query."
❌ Bad:
"Performance issues here."
Prioritize issues :
🔴 Critical: Security, data loss, major bugs
🟡 Important: Performance, maintainability
🟢 Nice-to-have: Style, minor improvements
Acknowledge good work :
"Nice use of the strategy pattern here! This makes it easy to add
new payment methods in the future."
Review checklist
Functionality
Code does what it's supposed to do
Edge cases handled
Error cases handled
No obvious bugs
Code Quality
Clear, descriptive naming
Functions are small and focused
No code duplication
Consistent with codebase style
No code smells
Security
Input validation
No hardcoded secrets
Authentication/authorization
No SQL injection vulnerabilities
No XSS vulnerabilities
Performance
No obvious bottlenecks
Efficient algorithms
Proper database queries
Resource management
Testing
Tests included
Good test coverage
Tests are maintainable
Edge cases tested
Documentation
Code is self-documenting
Comments where needed
Docs updated
Breaking changes documented
Common issues
Anti-patterns
God class :
# Bad: One class doing everything
class UserManager:
def create_user(self): pass
def send_email(self): pass
def process_payment(self): pass
def generate_report(self): pass
Magic numbers :
# Bad
if user.age > 18:
pass
# Good
MINIMUM_AGE = 18
if user.age > MINIMUM_AGE:
pass
Deep nesting :
# Bad
if condition1:
if condition2:
if condition3:
if condition4:
# deeply nested code
# Good (early returns)
if not condition1:
return
if not condition2:
return
if not condition3:
return
if not condition4:
return
# flat code
Security vulnerabilities
SQL Injection :
# Bad
query = f"SELECT * FROM users WHERE id = {user_id}"
# Good
query = "SELECT * FROM users WHERE id = %s"
cursor.execute(query, (user_id,))
XSS :
// Bad
element.innerHTML = userInput;
// Good
element.textContent = userInput;
Hardcoded secrets :
# Bad
API_KEY = "sk-1234567890abcdef"
# Good
API_KEY = os.environ.get("API_KEY")
Best practices
Review promptly : Don't make authors wait
Be respectful : Focus on code, not the person
Explain why : Don't just say what's wrong
Suggest alternatives : Show better approaches
Use examples : Code examples clarify feedback
Pick your battles : Focus on important issues
Acknowledge good work : Positive feedback matters
Review your own code first : Catch obvious issues
Use automated tools : Let tools catch style issues
