10 Bad Practices to Avoid in ASP.NET Core API Controllers
Tags: dotnetcore, csharp, programming
Published: 2023-10-18 07:24:09

Building APIs is like creating a bridge for different software components to communicate, and it can be a lot of fun. But, just like any other craft, there are some best practices and pitfalls to avoid. In this conversation, we'll take a friendly stroll through 10 common "oopsies" you should steer clear of when building ASP.NET Core API controllers.

Think of these as the friendly neighborhood guidelines that help you keep your API organized, secure, and easily understandable. We'll talk about everything from how to structure your API routes to handling errors with grace, so you can serve up smooth and reliable services to your users. So, let's get started on this API-building adventure!

**1. Not Using Attribute Routing:** Imagine your API is like a menu in a restaurant. You need to organize it by putting labels (attributes) on each dish (endpoint). Without these labels, it's like a menu with no sections, making it hard to find what you want to order.

```csharp
// Bad Practice
[Route("api/controller")]
public class MyController : Controller
{
    [HttpGet("getdata")]
    public IActionResult GetData()
    {
        // ...
    }
}

```

```csharp
// Good Practice
[Route("api")]
public class MyController : Controller
{
    [HttpGet("data")]
    public IActionResult GetData()
    {
        // ...
    }
}
```

**2. Exposing Sensitive Information:** When something goes wrong in your API, you don't want to give away too much information. It's like a detective not revealing all the clues. In your code, you should avoid telling users too much about the errors that occur.

```csharp
// Bad Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _dataService.GetDataById(id);
    if (data == null)
    {
        return NotFound($"Data with ID {id} not found.");
    }
    return Ok(data);
}

```

```csharp
// Good Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _dataService.GetDataById(id);
    if (data == null)
    {
        return NotFound();
    }
    return Ok(data);
}

```

**3. Overusing Data Annotations:** Think of data annotations like sticky notes on a form. It's better to have a single note (like a class for input) with all the information rather than putting sticky notes all over the place.

```csharp
// Bad Practice
[HttpPost]
public IActionResult Create([Required] string name, [Range(1, 100)] int age)
{
    // ...
}

```

```csharp
// Good Practice
[HttpPost]
public IActionResult Create([FromBody] UserData userData)
{
    if (!ModelState.IsValid)
    {
        return BadRequest(ModelState);
    }
    // ...
}

```

**4. Ignoring Input Validation:** It's like letting people enter any building without checking who they are. In your code, you should always check and make sure the data people give you is valid and safe.

```csharp
// Bad Practice
[HttpPost]
public IActionResult Create(UserData userData)
{
    // No input validation
    // ...
}

```

```csharp
// Good Practice
[HttpPost]
public IActionResult Create([FromBody] UserData userData)
{
    if (!ModelState.IsValid)
    {
        return BadRequest(ModelState);
    }
    // ...
}

```

**5. Overusing HTTP Verb Tunneling:** If you use the wrong method (like using a hammer to cut a cake), things can get confusing. Make sure you use the right method (HTTP verb) for each job in your API.

```csharp
// Bad Practice
[HttpPost("update/{id}")]
public IActionResult Update(int id, [FromBody] UpdateData data)
{
    // ...
}

```
```csharp
// Good Practice
[HttpPut("update/{id}")]
public IActionResult Update(int id, [FromBody] UpdateData data)
{
    // ...
}

```

**6. Not Using Dependency Injection:** Imagine you need tools to build a treehouse. Instead of asking for tools, you start building your own tools every time. That's what happens when you don't use dependency injection. You should ask for the tools you need.

```csharp
// Bad Practice
public MyController()
{
    _dataService = new DataService();
}

```

```csharp
// Good Practice
public MyController(IDataService dataService)
{
    _dataService = dataService;
}

```

**7. Not Handling Exceptions Gracefully:** If your API makes a mistake, it's like a chef burning a dish. You should apologize and offer something else (handle the error gracefully) instead of just serving a burnt dish (an error) to your customers.

```csharp
// Bad Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _dataService.GetDataById(id);
    return Ok(data); // No exception handling
}

```

```csharp
// Good Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    try
    {
        var data = _dataService.GetDataById(id);
        return Ok(data);
    }
    catch (Exception ex)
    {
        return StatusCode(500, "An error occurred.");
    }
}

```

**8. Not Using ViewModel/DTO:** When you show a house to someone, you don't show them all the messy details (domain models) inside. You use a clean and presentable version (ViewModel) of the house. Similarly, in your API, use ViewModel or DTO to present data to the user, not the messy stuff from your database.

```csharp
// Bad Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _dbContext.Users.Find(id);
    return Ok(data);
}

```

```csharp
// Good Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _userService.GetUserById(id);
    return Ok(data.ToViewModel());
}

```

**9. Not Implementing Proper Authentication and Authorization:** It's like having a nightclub with no bouncer at the door. Anyone can enter, and that can lead to trouble. In your API, you should have a "bouncer" (authentication and authorization) to check who's allowed in.

```csharp
// Bad Practice
[HttpGet("{id}")]
public IActionResult Get(int id)
{
    var data = _dataService.GetDataById(id);
    return Ok(data);
}

```

```csharp
// Good Practice
[HttpGet("{id}")]
[Authorize]
public IActionResult Get(int id)
{
    var data = _dataService.GetDataById(id);
    return Ok(data);
}

```


**10. Not Versioning Your API:** Imagine a phone that doesn't get updates. It can become outdated and stop working well. Similarly, your API needs updates too. You should create different versions (like software updates) of your API to keep it running smoothly for all users.

```csharp
// Bad Practice
[Route("api/controller")]
public class MyController : Controller
{
    [HttpGet("getdata")]
    public IActionResult GetData()
    {
        // ...
    }
}

```

```csharp
// Good Practice
[Route("api/v1/controller")]
public class MyController : Controller
{
    [HttpGet("getdata")]
    public IActionResult GetData()
    {
        // ...
    }
}

```
By avoiding these bad practices, you can make your API more organized, secure, and user-friendly, just like a well-managed restaurant or a smoothly running machine.