C#•14mo ago

✅ Recommended way to handle null results from a repository GET action?

I'm learning .NET Core and in Java (yes, I did Java) I used to create custom exceptions to throw. The equivalent would be

var company = _repository.Company.GetCompany(companyId, trackChanges);
if (company is null)
    throw new CompanyNotFoundException(companyId);

var company = _repository.Company.GetCompany(companyId, trackChanges);
if (company is null)
    throw new CompanyNotFoundException(companyId);

Is this a good idea?

20 Replies

Pobiega•14mo ago

Generally, no this is using exceptions for control flow, and should be avoided but it depends on your codebase if a query missing is never expected, then throwing might be the correct call but if you ever catch that error and handle it elsewhere, then I'd argue you should just have your GetCompany method return a Company? instead ie, make it clear that it can return nulls

SonathOP•14mo ago

basically this is the repo logic

public Company? GetCompany(Guid companyId, bool trackChanges) =>
            // #TODO is new Company a good way to handle null return?
            FindByCondition(company => 
                company.Id.Equals(companyId), trackChanges)
                .SingleOrDefault();

public Company? GetCompany(Guid companyId, bool trackChanges) =>
            // #TODO is new Company a good way to handle null return?
            FindByCondition(company => 
                company.Id.Equals(companyId), trackChanges)
                .SingleOrDefault();

idk if I should handle it differently using .SingleOrDefault() so if I make it clear that it can return nulls then I don't need conditional logic? but then how would I change the informational message that an entry was not found? Or is it not necessary? 👀

Pobiega•14mo ago

you never mentioned what kind of app this is but again, if the query can return null, meaning no company with that id, then just check for null at the appropriate level if finding by ID shouldn't be able to fail (like the ID is not from the user), then thats a different story

SonathOP•14mo ago

ah sorry it's a Web API

Pobiega•14mo ago

okay, but surely your controller/endpoint doesnt call the repository itself?

IsNotNull•14mo ago

In ASP.NET Core web controllers, you can return a not found response.

Pobiega•14mo ago

and even if it does, I'd check for null and return a 404 or bad request or whatever there

IsNotNull•14mo ago

To let the client know that the thing it requested doesn't exist

SonathOP•14mo ago

yeah so, basically the custom exception does that, I just think that (after talking with others too) it's better NOT to throw an exception here, and reserve exceptions for really exceptional things.

Erroneous Fatality•14mo ago

Your approach is absolutely fine. You can do a short-hand version:

Company company = _repository.Company.GetCompany(companyId, trackChanges)
?? throw new ArgumentException("The company was not found.", nameof(companyId));

Company company = _repository.Company.GetCompany(companyId, trackChanges)
?? throw new ArgumentException("The company was not found.", nameof(companyId));

Be careful not to create too many specialized exception classes, if you're not actually catching them anywhere. For your usecase, I'd suggest just using an ArgumentException, and having a middleware set up in your API which transforms ArgumentExceptions to 400 HTTP status code responses. Also, you might want to use async communication in your repository.

Pobiega•14mo ago

Using exceptions for control flow is not best practice.

Erroneous Fatality•14mo ago

Don't do .SingleOrDefault(), because that will make your queries more complicated. You should trust your Data set, your service code is the one maintaining it. It is absolutely fine.

Pobiega•14mo ago

No, its not.

Erroneous Fatality•14mo ago

Do .FirstOrDefault() instead, no need to check on every query if your dataset is faulty.

SonathOP•14mo ago

the consumer may query for a company that doesn't exist

Pobiega•14mo ago

handle the null appropriately and its not an issue.

SonathOP•14mo ago

yeah I think I agree with Pobiega, I'll remove the conditional checking and handle it differently

Erroneous Fatality•14mo ago

Using exceptions for exceptional cases is fine. I've made 10+ highly concurrent web apps in my career using this approach, haven't had a situation where I needed to optimize that. Alright, peace

Pobiega•14mo ago

Its not about performance. its about intention and clarity of code. its not exceptinal that a query returned no results.. jesus christ.

Accord•14mo ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

✅ Recommended way to handle null results from a repository GET action?