clean-code-dotnet

Use Meaningful Names

```csharp

// ❌ Bad

int d;

var dataFromDb = db.GetFromService().ToList();

// ✅ Good

int daySinceModification;

var employees = _employeeService.GetEmployees().ToList();

```

Avoid Hungarian Notation

```csharp

// ❌ Bad

int iCounter;

string strFullName;

public bool IsShopOpen(string pDay, int pAmount) { }

// ✅ Good

int counter;

string fullName;

public bool IsShopOpen(string day, int amount) { }

```

Use Pronounceable Names

```csharp

// ❌ Bad

public class Employee

{

public DateTime sWorkDate { get; set; }

public DateTime modTime { get; set; }

}

// ✅ Good

public class Employee

{

public DateTime StartWorkingDate { get; set; }

public DateTime ModificationTime { get; set; }

}

```

Use Domain Names

```csharp

// ✅ Good - Use patterns developers know

var singletonObject = SingleObject.GetInstance();

var factory = new PatientFactory();

var repository = new PatientRepository();

```

---

Return Early, Avoid Deep Nesting

```csharp

// ❌ Bad - Deep nesting

public bool IsShopOpen(string day)

{

if (!string.IsNullOrEmpty(day))

{

day = day.ToLower();

if (day == "friday")

{

return true;

}

else if (day == "saturday")

{

return true;

}

// ... more nesting

}

return false;

}

// ✅ Good - Guard clauses + early return

public bool IsShopOpen(string day)

{

if (string.IsNullOrEmpty(day))

return false;

var openingDays = new[] { "friday", "saturday", "sunday" };

return openingDays.Contains(day.ToLower());

}

```

Avoid Magic Strings

```csharp

// ❌ Bad

if (userRole == "Admin") { }

// ✅ Good

const string AdminRole = "Admin";

if (userRole == AdminRole) { }

// ✅ Better - Use enum

public enum UserRole { Admin, User, Guest }

if (userRole == UserRole.Admin) { }

```

Don't Add Unneeded Context

```csharp

// ❌ Bad - Redundant prefix

public class Car

{

public string CarMake { get; set; }

public string CarModel { get; set; }

public string CarColor { get; set; }

}

// ✅ Good

public class Car

{

public string Make { get; set; }

public string Model { get; set; }

public string Color { get; set; }

}

```

Use Default Arguments

```csharp

// ❌ Bad

public void CreateMicrobrewery(string name = null)

{

var breweryName = !string.IsNullOrEmpty(name) ? name : "Hipster Brew Co.";

}

// ✅ Good

public void CreateMicrobrewery(string breweryName = "Hipster Brew Co.")

{

// breweryName is always valid

}

```

---

Functions Should Do One Thing

```csharp

// ❌ Bad - Multiple responsibilities

public void SendEmailToListOfClients(string[] clients)

{

foreach (var client in clients)

{

var clientRecord = db.Find(client);

if (clientRecord.IsActive())

{

Email(client);

}

}

}

// ✅ Good - Single responsibility

public void SendEmailToActiveClients(string[] clients)

{

var activeClients = GetActiveClients(clients);

activeClients.ForEach(client => Email(client));

}

public List GetActiveClients(string[] clients)

{

return db.Find(clients).Where(c => c.IsActive).ToList();

}

```

Avoid Side Effects

```csharp

// ❌ Bad - Modifies global state

var name = "Ryan McDermott";

public void SplitAndEnrichFullName()

{

var temp = name.Split(" ");

name = $"First: {temp[0]}, Last: {temp[1]}"; // Side effect!

}

// ✅ Good - Pure function

public string SplitAndEnrichFullName(string name)

{

var temp = name.Split(" ");

return $"First: {temp[0]}, Last: {temp[1]}";

}

```

Avoid Negative Conditionals

```csharp

// ❌ Bad

public bool IsDOMNodeNotPresent(string node) { }

if (!IsDOMNodeNotPresent(node)) { } // Double negative!

// ✅ Good

public bool IsDOMNodePresent(string node) { }

if (IsDOMNodePresent(node)) { }

```

Avoid Flag Parameters

```csharp

// ❌ Bad - Flag indicates multiple responsibilities

public void CreateFile(string name, bool temp = false)

{

if (temp)

Touch("./temp/" + name);

else

Touch(name);

}

// ✅ Good - Separate methods

public void CreateFile(string name) => Touch(name);

public void CreateTempFile(string name) => Touch("./temp/" + name);

```

Limit Function Arguments (2 or fewer)

```csharp

// ❌ Bad

public void CreateMenu(string title, string body, string buttonText, bool cancellable) { }

// ✅ Good - Use object

public class MenuConfig

{

public string Title { get; set; }

public string Body { get; set; }

public string ButtonText { get; set; }

public bool Cancellable { get; set; }

}

public void CreateMenu(MenuConfig config) { }

```

Encapsulate Conditionals

```csharp

// ❌ Bad

if (article.state == "published") { }

// ✅ Good

if (article.IsPublished()) { }

```

Remove Dead Code

```csharp

// ❌ Bad

public void OldRequestModule(string url) { } // Unused!

public void NewRequestModule(string url) { }

var request = NewRequestModule(requestUrl);

// ✅ Good - Delete unused code

public void RequestModule(string url) { }

var request = RequestModule(requestUrl);

```

---

Single Responsibility (SRP)

```csharp

// ❌ Bad - Two responsibilities

class UserSettings

{

public void ChangeSettings(Settings settings)

{

if (VerifyCredentials()) { / ... / }

}

private bool VerifyCredentials() { / ... / } // Auth responsibility

}

// ✅ Good - Separated

class UserAuth

{

public bool VerifyCredentials() { / ... / }

}

class UserSettings

{

private readonly UserAuth _auth;

public void ChangeSettings(Settings settings)

{

if (_auth.VerifyCredentials()) { / ... / }

}

}

```

Open/Closed (OCP)

```csharp

// ❌ Bad - Must modify to extend

class HttpRequester

{

public bool Fetch(string url)

{

if (adapterName == "ajaxAdapter")

return MakeAjaxCall(url);

else if (adapterName == "httpNodeAdapter")

return MakeHttpCall(url);

// Must add more else-if for new adapters!

}

}

// ✅ Good - Open for extension, closed for modification

interface IAdapter

{

bool Request(string url);

}

class AjaxAdapter : IAdapter

{

public bool Request(string url) { / ... / }

}

class HttpRequester

{

private readonly IAdapter _adapter;

public bool Fetch(string url) => _adapter.Request(url);

}

```

Liskov Substitution (LSP)

```csharp

// ❌ Bad - Square breaks Rectangle behavior

class Square : Rectangle

{

public override void SetWidth(double width) { Width = Height = width; }

}

// ✅ Good - Use abstraction

abstract class Shape

{

public abstract double GetArea();

}

class Rectangle : Shape { / ... / }

class Square : Shape { / ... / }

```

Interface Segregation (ISP)

```csharp

// ❌ Bad - Robot can't eat but must implement

interface IEmployee { void Work(); void Eat(); }

class Robot : IEmployee

{

public void Work() { / ... / }

public void Eat() { / Robot can't eat! / }

}

// ✅ Good - Segregated interfaces

interface IWorkable { void Work(); }

interface IFeedable { void Eat(); }

class Human : IWorkable, IFeedable { / ... / }

class Robot : IWorkable { / ... / }

```

Dependency Inversion (DIP)

```csharp

// ❌ Bad - Depends on concrete types

class Manager

{

private readonly Robot _robot;

private readonly Human _human;

}

// ✅ Good - Depends on abstractions

class Manager

{

private readonly IEnumerable _employees;

public Manager(IEnumerable employees)

{

_employees = employees;

}

}

```

Constructor Dependency Smell (SRP Indicator)

Too many constructor dependencies indicate SRP violation:

```csharp

// ❌ Code Smell: 15 dependencies = too many responsibilities!

public class LicensePlateAppService : ApplicationService

{

public LicensePlateAppService(

IRepository licensePlateRepository,

IRepository licensePlateWithoutTagRepository,

IRepository asnRepository,

IRepository projectRepository,

IRepository tagRepository,

IRepository skuRepository,

IRepository customerRepository,

IRepository licensePlateHoldRepository,

IRepository licensePlateLocationRepository,

IRepository locationRepository,

IWarehouseAppService warehouseAppService,

IWarehouseOwnerAppService warehouseOwnerAppService,

IBlobContainer fileContainer,

LicensePlateService.LicensePlateServiceClient licensePlateServiceClient,

CommonDependencies commonDependencies)

{ }

}

// ✅ Good: Split by responsibility

public class LicensePlateAppService { } // CRUD only (~5 deps)

public class LicensePlateBulkService { } // Bulk imports (~4 deps)

public class LicensePlateEventPublisher { } // Events (~3 deps)

```

Dependency Count Guidelines:

| Dependencies | Status | Action |

|--------------|--------|--------|

| 1-5 | ✅ Normal | Acceptable |

| 6-8 | ⚠️ Warning | Review for splitting opportunities |

| 9+ | ❌ Smell | Refactor required - class has too many responsibilities |

Refactoring Strategies:

1. Extract Service - Move related operations to a dedicated service
2. Facade Pattern - Group related dependencies behind a facade
3. Domain Events - Decouple via publish/subscribe instead of direct calls
4. Mediator Pattern - Use MediatR to reduce direct dependencies

---

Don't Use `throw ex`

```csharp

// ❌ Bad - Loses stack trace

catch (Exception ex)

{

logger.LogError(ex);

throw ex; // Stack trace lost!

}

// ✅ Good - Preserves stack trace

catch (Exception ex)

{

logger.LogError(ex);

throw; // Rethrows with original stack

}

// ✅ Also Good - Wrap with inner exception

catch (Exception ex)

{

throw new BusinessException("Operation failed", ex);

}

```

Don't Ignore Caught Errors

```csharp

// ❌ Bad - Silent swallow

catch (Exception ex) { } // Never do this!

// ✅ Good - Handle or propagate

catch (Exception ex)

{

_logger.LogError(ex, "Operation failed");

throw; // Or handle appropriately

}

```

Use Multiple Catch Blocks

```csharp

// ❌ Bad - Type checking in catch

catch (Exception ex)

{

if (ex is TaskCanceledException) { / ... / }

else if (ex is TaskSchedulerException) { / ... / }

}

// ✅ Good - Separate catch blocks

catch (TaskCanceledException ex)

{

// Handle cancellation

}

catch (TaskSchedulerException ex)

{

// Handle scheduler error

}

```

---

Avoid Positional Markers and Regions

```csharp

// ❌ Bad

#region Scope Model Instantiation

var model = new Model();

#endregion

#region Action setup

void Actions() { }

#endregion

// ✅ Good - Let code speak

var model = new Model();

void Actions() { }

```

Don't Leave Commented Code

```csharp

// ❌ Bad

DoStuff();

// DoOtherStuff();

// DoSomeMoreStuff();

// ✅ Good - Use version control

DoStuff();

```

Only Comment Business Logic Complexity

```csharp

// ❌ Bad - Obvious comments

var hash = 0; // The hash

var length = data.Length; // Length of string

// ✅ Good - Explains WHY, not WHAT

// Using djb2 hash for good speed/collision tradeoff

hash = ((hash << 5) - hash) + character;

```

---

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/solid-principles.md: Full SOLID examples
• references/async-patterns.md: Async/await guidelines
• references/editorconfig-template.md: .editorconfig template

Source: [clean-code-dotnet](https://github.com/thangchung/clean-code-dotnet)