Shuai's Code Review Style

Analysis of Shuai’s code review style based on 200+ comments across 50 MRs. Use this guide for self-review before submission or to conduct reviews in a similar style.

Core Focus Areas

1. Type Safety & PHPStan (Primary Focus)

Shuai strongly advocates for proper type annotations and avoiding @phpstan-ignore.

Principle: Fix the root cause instead of suppressing errors.

Examples:

“Should narrow the type of $source_object->investment_account->subtype->value instead of using @phpstan-ignore”

“If PHPStan cannot identify the type of $person, the better approach is add proper PHPDoc to help PHPStan to identify it.”

“You should check is the $property int $cash_value correct instead of using @phpstan-ignore”

“There should be a better way to solve this PHPStan error”

“Instead of using @phpstan-ignore, I prefer add PHPDoc to $due_tasks_by_household_id”

“这个 rule 不应该被 ignore，“子类声明参数的类型比父类声明的更精准” 是有问题的。了解下”里氏替换原则” 和 “协变/逆变""

2. Laravel Best Practices

Shuai ensures code follows Laravel conventions and uses built-in features.

Examples:

“Recommend use associate()” (instead of manually setting foreign keys)

“Why not use Readable trait?” / “Why not use Updatable trait?”

“In Retail, facade DB is generally used.” (prefer facades over DI)

“Use const ModelWritten::EVENT_CREATED”

“For this scenario, better approach is extract a new Request class, like ListRequst”

“这段逻辑重复了多次，可以考虑在 Household 中增加一个 Local Scope”

“Should use transaction”

3. Code Simplification

Shuai frequently identifies unnecessary complexity.

Examples:

“This method do nothing, why need this method?”

“没必要单独声明一个变量”

“这个中间变量可以移除了”

“Unnecessary PHPDoc”

“getQuery() isn’t necessary”

“这种没有额外信息的 PHPDoc 不需要添加”

“可以省略的 phpdoc”

4. Naming Conventions

Shuai pays attention to consistent naming patterns.

Examples:

“variable name should be snake case”

“This name is not proper, T* is generic style”

“$engine_object => $projection_info_engine_object”

“We generally don’t use nouns as the key of bool values, I think use_todays_dollars is better name.”

“SUFFIX_TODAYS_DOLLARS, 一般把常量的类型放在前面。”

“Replace ‘household_id’ with const”

5. RESTful Design

Shuai values consistent API design patterns.

Examples:

“I think there are more benefits to following the same RESTful style than saving one ID in the URL.”

“Do you missing setup? I think operations/advisors.households.setup.store is ok.”

6. N+1 Query Prevention

Examples:

“Potential n+1 query, should eager load people relation”

“我理解这种写法是不想使用 LocalityManager::getHouseholdOrFail(), 但是这样会有 n+1 query”

“通过 HomeController 调用到这个方法时也可能出现 n+1，并且不好通过预加载解决。“

7. Exception Handling

Shuai emphasizes correct exception types for different scenarios.

Examples:

“应该抛出 LogicException, UnprocessableEntityHttpException 通常在 request 中含有 invalid 数据时抛出”

“不合适的异常类型”

“Should be LogicalException”

8. Proper Abstraction Boundaries

Shuai identifies when abstractions are misplaced.

Examples:

“如果一个 Trait 内全部是静态方法，它不适合被提取成 Trait”

“All methods in this file is static method, so trait should not be used.”

“这个方法不能继续放在这个 trait 了。因为 TaxEfficientCategory 也用到了这个方法…”

“我建议是先放 TweakController，等之后真的要复用了，再修改成更通用的版本提取到 Household”

9. Event/Listener Patterns

Examples:

“I think trigger a event is better than directly invoke listener logic”

“在 command 的上下文里，我觉得直接插入数据比使用 event/listener 更容易理解和维护”

“In this case, consider trigger TaskUpdated in App\Models\Events\TaskEvent”

Self-Review Checklist

Before submitting for review, check these items:

Type Safety

• No @phpstan-ignore unless absolutely necessary
• PHPDoc types match actual runtime behavior
• Use @phpstan-type for complex repeated array shapes
• Nullable types are intentional and documented
• Generic types are properly constrained

Laravel Patterns

• Use associate() for relationship assignments
• Use Readable/Updatable traits when applicable
• Use facades (DB, Auth) consistently
• Extract dedicated Request classes for complex validation
• Use Local Scopes for repeated query logic
• Wrap related operations in transactions
• Use model constants instead of string literals

Code Quality

• Remove unnecessary intermediate variables
• Remove PHPDoc that just repeats type hints
• Remove methods that do nothing
• Remove unused parameters
• Simplify complex expressions when possible

Naming

• Variables use snake_case
• Constants have type prefix (e.g., SUFFIX_*, PREFIX_*)
• Boolean fields use verb prefixes (use_*, is_*, has_*)
• Use constants instead of string literals for keys
• Generic type parameters use T prefix

Database & Performance

• Eager load relationships to prevent N+1
• Use $table->timestamp('created_at')->useCurrent() for simpler migrations
• Data migrations use data_ prefix in filename
• Consider executeWithLockAndCircuitBreaker for long operations

API Design

• Follow RESTful conventions
• Include related entities in responses when useful
• Validate query parameters (date format, etc.)

Exceptions

• Use LogicException for programming errors
• Use BadRequestHttpException for invalid user input
• Use UnprocessableEntityHttpException for semantic validation failures

Testing

• Tests cover edge cases
• Migration scripts are tested
• Don’t add excessive test code

Abstraction

• Traits have instance methods (not all static)
• Keep methods in appropriate classes until reuse is proven
• Don’t over-engineer for hypothetical future needs

Review Style Characteristics

Trait	Example
Focus on type safety	”Should narrow the type instead of using @phpstan-ignore”
Laravel expertise	”Recommend use associate()“
Simplification focus	”这个中间变量可以移除了”
Pragmatic	”I think the current position is not ideal, but still better than pure string literals”
Bilingual	Uses both English and Chinese naturally
Constructive	Often provides code suggestions with rationale
Questions purpose	”When do developers need to use getSourceObject instead of getSourceObjectOrFail?”
References docs	Links to Laravel documentation for patterns

Common Phrases

• “Unnecessary…” / “没必要…” — Identifying redundant code
• “I prefer…” / “我倾向于…” — Suggesting alternatives
• “Should use…” — Prescriptive corrections
• “ditto” — Same comment applies to similar code
• “Why not use…?” — Suggesting existing solutions
• “This approach is more simpler” — Advocating simplicity
• “可以考虑…” — Gentle suggestions

Patterns to Avoid

Based on Shuai’s reviews, avoid these patterns:

1. Suppressing PHPStan errors without fixing root cause
2. AI-generated comments that add no value
3. Static-only traits (should be regular classes or functions)
4. Manual foreign key assignment (use associate())
5. String literals for model columns (use constants)
6. Eager abstraction before reuse is proven
7. Missing type declarations on closures and callbacks
8. Excessive test code that doesn’t add coverage value

Summary

Shuai’s review style is pragmatic and type-focused, emphasizing:

1. Type safety — Fix PHPStan errors properly, don’t suppress them
2. Laravel conventions — Use built-in traits, facades, and patterns
3. Simplification — Remove unnecessary variables, methods, and PHPDoc
4. Consistent naming — Follow snake_case, use constants, proper prefixes
5. Performance awareness — Prevent N+1 queries, use eager loading
6. Proper exceptions — Match exception type to error category
7. Pragmatic abstraction — Don’t over-engineer, prove reuse first