12 Bad Practices to Avoid in ASP.NET Core API Controllers

1. Tight Coupling

Avoid tight coupling in controllers with specific dependencies. Instead, use dependency injection to inject the dependencies into controllers. Which is more maintainable and testable.

// Avoid
[ApiController]
[Route("[controller]")]
public class ProductController : ControllerBase
{
    ProductService productService = new ProductService();
    // ...
}

// Prefer:
[ApiController]
[Route("[controller]")]
public class ProductController : ControllerBase
{
    private readonly ILogger<ProductController> _logger;
    private readonly IProductService _productService;

    public ProductController(IProductService productService, ILogger<ProductController> logger)
    {
        _logger = logger;
        _productService = productService;
    }
    // ...
}        

2. Mixing Concerns

The controller should focus on HTTP requests and generating responses. Avoid mixing concerns such as authentication, authorization or any other validation instead use middleware or any separate class or service.

// Avoid
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    // Authentication and authorization logic here
    // ...
    // Data access logic here
    // ...
    return Ok(StatusCodes.Status201Created);
}


// Prefer:
[HttpPost]
[Authorize]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    await _productService.CreateAsync(productDto);

    return Ok(StatusCodes.Status201Created);
}        

3. Lack of Exception Handling

Avoid using a try-catch block in the controller instead use exception middleware to better handle exceptions to return generic error messages.

// Avoid (Inconsistent error handling or try catch everywhere
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    try
    {
        await _productService.CreateAsync(productDto);

        return Ok(StatusCodes.Status201Created);
    }
    catch (ProductValidationException ex)
    {
        return BadRequest(ex.Message);
    }
    catch (Exception ex)
    {
        _logger.LogError(ex, "An error occurred while creating the product.");
        return StatusCode(StatusCodes.Status500InternalServerError);
    }
}


// Prefer Exception filters or Middleware
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    await _productService.CreateAsync(productDto);

    return Ok(StatusCodes.Status201Created);
}        

4. Long Running Operations

Avoid performing long-running operations in controllers. Instead, use enqueue for processing in the background to avoid any halt in the system.

// Avoid
[HttpGet]
public async Task<IActionResult> GenerateReport(Report report)
{
    // Long-running operation
    // ...
    // ...

    return Ok(report);
}


// Prefer
[HttpPost]
public async Task<IActionResult> GenerateReport(Report report)
{
    var taskIdentifier = await _messageQueueService.EnqueueAsync(report);

    return StatusCode(StatusCodes.Status202Accepted, taskIdentifier);
}        

5. Lack of Validation

Input validation is crucial to ensure the integrity and security of the system. Avoid neglecting input validation in your controllers.

// Avoid
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    // No validation
    await _productService.CreateAsync(productDto);
    return Ok(StatusCodes.Status201Created);
}


// Prefer:
[HttpPost]
public async Task<IActionResult> CreateProduct([FromBody] ProductDto productDto)
{
    if (!ModelState.IsValid)
    {
        return StatusCode(StatusCodes.Status400BadRequest, ModelState);                
    }

    await _productService.CreateAsync(productDto);
    return Ok(StatusCodes.Status201Created);
}        

6. Direct Database Access

Avoid direct database access instead use any service, or repository to decouple the controllers from specific data access technologies.

// Avoid
[HttpGet]
public IActionResult GetProduct(int productId)
{
    var product = dbContext.Products.Find(productId);
    return Ok(product);
}


// Prefer:
[HttpGet]
public async Task<IActionResult> GetProduct(int productId)
{
    var product = await _productService.GetByIdAsync(productId);
    return Ok(product);
}        

7. Lack of Caching

Implement caching mechanisms when appropriate. Leverage caching to improve performance and reduce load on the server.

// Avoi
[HttpGet]
public async Task<IActionResult> GetProducts()
{
    var products = await _productService.GetAllAsync();
    return Ok(products);
}


// Prefer
[HttpGet]
[ResponseCache(Duration = 60)] // Cache the response for 60 seconds
public async Task<IActionResult> GetProducts()
{
    var products = await _productService.GetAllAsync();
    return Ok(products);
}        

8. Lack of Authentication and authorization

Implement authentication and authorization for sensitive operations. Secure controllers and actions method accordingly.

// Avoi
[HttpPost]
public async Task<IActionResult> DeleteProduct(int productId)
{
    // No authentication or authorization
    await _productService.DeleteAsync(productId);
    return StatusCode(StatusCodes.Status200OK);
}


// Prefer
[HttpPost]
[Authorize(Roles = "Admin")]
public async Task<IActionResult> DeleteProduct(int productId)
{
    await _productService.DeleteAsync(productId);
    return StatusCode(StatusCodes.Status200OK);
}        

9. Excessive Logic

Avoid excessive logic. The controller should primarily be responsible for handling incoming requests and returning responses. For any complex logic use any separate utility or services.

// Avoid
[HttpGet]
public async Task<IActionResult> GetProducts()
{
    // Complex business logic here
    // ...
    // ...
    return Ok(products);
}


// Prefer:
[HttpGet]
public async Task<IActionResult> GetProducts()
{
    var products = await _productService.GetAllAsync();
    return Ok(products);
}        

10. Ignoring HTTP verbs and RESTful principles

Controllers in ASP.NET Core should adhere to the principles of RESTful architecture. Avoid using improper HTTP verbs or actions that don't align with RESTful conventions. Use appropriate HTTP verbs (GET, POST, PUT, DELETE, etc).

// Avoid
public IActionResult DeleteProduct(int productId)
{
    // ...
}


// Prefer:
[HttpDelete("/api/products/{id}")]
public IActionResult DeleteProduct(int productId)
{
    // ...
}        

11. Lack of proper Routing

Ensure that the controller is properly routed to handle incoming requests. Avoid inconsistent or ambiguous routing configurations.

// Avoid
[HttpGet]
public IActionResult Get()
{
    // ...
}


// Prefer:
[HttpGet("api/products")]
public IActionResult Get()
{
    // ...
}        

12. Lack of Logging

Logging is a very crucial aspect of application development, as it helps track important events, conditions, and errors during the execution of our code. Use middleware or action filters to capture relevant information.

// Avoi
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
    if (someSimpleCondition)
    {
        // ...
    }

    await _productService.CreateAsync(productDto);
    return Ok();
}


// Prefer
[HttpPost]
public async Task<IActionResult> CreateProduct(ProductDto productDto)
{
     if (someSimpleCondition)
     {
        // ...
        _logger.LogWarning("Warning: Some simple condition is met."); // Log a warning
     }

     await _productService.CreateAsync(productDto);
     return Ok();
}d