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

When developing controllers in ASP.NET Core, there are certain practices that should be avoided to ensure maintainability, performance, and adherence to best practices. Here are 10 things we should avoid in our controllers.

1. Tight coupling

Avoid tightly coupling in controllers with specific dependencies or frameworks. Instead, use dependency injection to inject dependencies into controllers. This promotes loose coupling and makes the code more testable and maintainable.

// 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;
    }
    // ...
}

Learn more about dependency injection

2. Mixing concerns

Controllers should focus on handling HTTP requests and generating responses. Avoid mixing concerns such as data access, authentication, or authorization directly in the controller. Delegate these responsibilities to separate classes or middleware.

// 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);
}

Learn more about the Single-responsibility principle | Separation of concerns

3. Lack of exception handling

Controllers should handle exceptions gracefully and return appropriate error responses. Avoid letting exceptions propagate to the client without proper handling. Implement global exception-handling mechanisms, such as exception filters or middleware, to centralize error handling.

// 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);
}

E.g of a global exception middleware

public class ExceptionMiddleware
{
    private readonly RequestDelegate _next;

    public ExceptionMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    public async Task InvokeAsync(HttpContext context)
    {
        try
        {
            await _next(context);
        }
        catch (Exception ex)
        {
            // Log the error
            // ...

            await HandleExceptionAsync(context, ex);
        }
    }

    private Task HandleExceptionAsync(HttpContext context, Exception exception)
    {
        context.Response.ContentType = "application/json";
        context.Response.StatusCode = (int)HttpStatusCode.InternalServerError;

        var response = new
        {
            StatusCode = context.Response.StatusCode,
            Message = "Internal Server Error"
        };

        return context.Response.WriteAsync(JsonConvert.SerializeObject(response));
    }
}

With this exception middleware, any unhandled exceptions that occur during the processing of an HTTP request will be caught, and a JSON response with a generic error message and the status code 500 (Internal Server Error) will be returned to the client.

We can customize the HandleExceptionAsync method to include more detailed error information or modify the response structure as per our requirements.

To use this middleware in our ASP.NET Core Web API application, we can add the following code to our Startup.cs file or Program.cs .net 6.

app.UseMiddleware<ExceptionMiddleware>();

Learn more about Exception filters | Middleware

4. Long-running operations

Avoid performing long-running operations synchronously in controllers. Offload such tasks to background workers or use asynchronous programming.

// Avoid:
[HttpGet]
public IActionResult GenerateReport()
{
    // Long-running operation
    // ...
    return Ok(report);
}


// Prefer:
[HttpGet]
public async Task<IActionResult> GenerateReport()
{
    // Offload the long-running operation to a background worker
    await _reportGenerationService.GenerateAsync();
    return Ok();
}

Learn more about Background tasks | Hangfire scheduler | Task asynchronous

5. Lack of validation

Input validation is crucial to ensure the integrity and security of the application. Avoid neglecting input validation in your controllers. Leverage model validation attributes, such as [Required], [MaxLength], and custom validation logic to validate incoming data.

// 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);
}

Learn more about Model Validation | FluentValidation

6. Direct database access

Avoid directly accessing the database from the controllers. Instead, use an abstraction layer, such as repositories or data access services, to decouple the controllers from specific data access technologies. This improves testability and maintainability.

// 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);
}

Learn more about Repository Pattern or Data Layer

7. Lack of caching

Avoid not utilizing caching mechanisms when appropriate. Leverage caching to improve performance and reduce load on the server.

// Avoid:
[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);
}

Learn more about Caching | Response caching

8. Lack of authentication and authorization

Avoid not implementing authentication and authorization for sensitive operations. Secure the controllers accordingly.

// Avoid:
[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);
}

Learn more about Authentication and Authorization

9. Excessive logic

Controllers should primarily be responsible for handling incoming requests and returning responses. They should not contain complex business logic. Instead, delegate the business logic to dedicated service classes or other components.

// 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);
}

Learn more about Common web application architectures

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.) to perform corresponding operations on resources.

// Avoid:
public IActionResult DeleteProduct(int id)
{
    // ...
}


// Prefer:
[HttpDelete("/api/products/{id}")]
[ProducesResponseType(StatusCodes.Status200OK)]
[Authorize]
public IActionResult DeleteProduct(int id)
{
    // ...
}

Learn more about RESTful web API design

By avoiding these common pitfalls, we can develop controllers in ASP.NET Core that are maintainable, testable, and follow best practices for API web development.

Thanks for reading!

Through my articles, I share Tips & Experiences on web development, career, and the latest tech trends. Join me as we explore these exciting topics together. Let’s learn, grow, and create together!

➕More article about Programming, Careers, and Tech Trends.

🔔 Follow me on Medium | Twitter