// ❌ BadpublicclassEmployee
{
public DateTime sWorkDate { get; set; }
public DateTime modTime { get; set; }
}
// ✅ GoodpublicclassEmployee
{
public DateTime StartWorkingDate { get; set; }
public DateTime ModificationTime { get; set; }
}
Use Domain Names
// ✅ Good - Use patterns developers knowvar singletonObject = SingleObject.GetInstance();
var factory = new PatientFactory();
var repository = new PatientRepository();
Variables
Return Early, Avoid Deep Nesting
// ❌ Bad - Deep nestingpublicboolIsShopOpen(string day)
{
if (!string.IsNullOrEmpty(day))
{
day = day.ToLower();
if (day == "friday")
{
returntrue;
}
elseif (day == "saturday")
{
returntrue;
}
// ... more nesting
}
returnfalse;
}
// ✅ Good - Guard clauses + early returnpublicboolIsShopOpen(string day)
{
if (string.IsNullOrEmpty(day))
returnfalse;
var openingDays = new[] { "friday", "saturday", "sunday" };
return openingDays.Contains(day.ToLower());
}
Refactor required - class has too many responsibilities
Refactoring Strategies:
Extract Service - Move related operations to a dedicated service
Facade Pattern - Group related dependencies behind a facade
Domain Events - Decouple via publish/subscribe instead of direct calls
Mediator Pattern - Use MediatR to reduce direct dependencies
Error Handling
Don't Use throw ex
// ❌ Bad - Loses stack tracecatch (Exception ex)
{
logger.LogError(ex);
throw ex; // Stack trace lost!
}
// ✅ Good - Preserves stack tracecatch (Exception ex)
{
logger.LogError(ex);
throw; // Rethrows with original stack
}
// ✅ Also Good - Wrap with inner exceptioncatch (Exception ex)
{
thrownew BusinessException("Operation failed", ex);
}
Don't Ignore Caught Errors
// ❌ Bad - Silent swallowcatch (Exception ex) { } // Never do this!// ✅ Good - Handle or propagatecatch (Exception ex)
{
_logger.LogError(ex, "Operation failed");
throw; // Or handle appropriately
}
Use Multiple Catch Blocks
// ❌ Bad - Type checking in catchcatch (Exception ex)
{
if (ex is TaskCanceledException) { /* ... */ }
elseif (ex is TaskSchedulerException) { /* ... */ }
}
// ✅ Good - Separate catch blockscatch (TaskCanceledException ex)
{
// Handle cancellation
}
catch (TaskSchedulerException ex)
{
// Handle scheduler error
}
Comments
Avoid Positional Markers and Regions
// ❌ Bad#region Scope Model Instantiationvar model = new Model();
#endregion#region Action setupvoidActions() { }
#endregion// ✅ Good - Let code speakvar model = new Model();
voidActions() { }
Don't Leave Commented Code
// ❌ Bad
DoStuff();
// DoOtherStuff();// DoSomeMoreStuff();// ✅ Good - Use version control
DoStuff();
Only Comment Business Logic Complexity
// ❌ Bad - Obvious commentsvar hash = 0; // The hashvar length = data.Length; // Length of string// ✅ Good - Explains WHY, not WHAT// Using djb2 hash for good speed/collision tradeoff
hash = ((hash << 5) - hash) + character;
Quick Reference Checklist
Code Review Checklist
Naming: Meaningful, pronounceable, no Hungarian
Functions: Single responsibility, <3 args, no flags
Variables: No magic strings, early returns, no nesting >2
SOLID: Interfaces over concrete, small focused classes
Dependencies: Constructor has <8 dependencies (SRP indicator)
Error Handling: No throw ex, no silent catch, specific exception types
Comments: No regions, no dead code, explains WHY
References
references/solid-principles.md: Full SOLID examples