- Predicates give a lot of control, but possibly more than is necessary.
- Named tuples solve the issue of returning a boolean and a string nicely.
Out parameters are inferior to named tuples.
- Enums are only usable internally, not as a return value.
- Exceptions are more acceptable than you suggest.
- (Not mentioned in the answer body) if you already have existing validation logic in other places of the codebase, it might be nicer to reuse the pattern as a matter of consistency.
public IEnumerable<Customer> Get(Expression<Func<Customer, bool>> predicate)
It looks like this approach leaks business specific rules to a controller and the point of having services is to keep those rules inside services.
It very much depends on what logic you're expecting. Some clear cut examples:
- Logic to hide soft-deleted customers should indeed not be the responsibility of the controller. It should be implemented in a repository layer.
- Logic to find all customers named "Bob" is part of the controller's duties, assuming that it's handling a request which contains a customer name filter.
Now, allowing a predicate to be passed this way gives your controller a lot of control. This has its upsides and downsides:
- Pro You don't spend time defining an intermediary layer where every filter/property/method is spelled out.
- Pro Less effort when refactoring queries.
- Con More effort when refactoring entities.
- Con Less separation of concerns.
This is a subjective decision. Different application have different needs and different focuses. If you have many different filters on an entity list, the cost of having to develop (and maintain) the intermediary layer increases.
You also run into issues such as having too many parameters in a method, or having to maintan method-specific classes to contain the parameters (which, in my opinion, is simply hiding the issue with having many parameters as opposed to fixing it).
public class ItemService
public bool CanDeleteItem(int itemId)
// Check if item can be deleted.
But now, how do I return reasons for why this item cannot be deleted?
Named tuples to the rescue here. I don't use them very often, because I'm apprehensive of being overly liberal with them.
However, this is one of those cases where the benefits vastly outweigh the potential for abuse:
public (bool reponse, string message) CanDeleteItem(int itemId)
// Check if item can be deleted.
Its usage is rather elegant:
(bool canBeDeleted, string errorMessage) = ItemService.CanDeleteItem(123);
Alternatively, if you're not a fan of named tuples, you can create your own
Response method that has a bool and a string property. The principle is the same.
One thing to note though: this approach can become cumbersome if errors can occur on multiple levels. It requires constant wrapping, and every method might even need its own
if wrapping to check the previous level's returned value.
This is actually where exceptions are more relevant to your case, because they are able to cross multiple levels without needing you to manually handle them every step of the way.
I could add another parameter
out string failReason, but this is not very meaningful and someone exposed to this method has no idea if this message will contain technical reasons or something that can be presented to end user.
Since the latest improvements to tuples (syntactically simpler, named fields), I would argue that they are a better substitute for
out parameters were always a rather hacky way of returning multiple values.
It makes no syntactical sense to define an additional return value in the input parameters (
out variable). It makes a lot more sense to defined an additional return value in the return type (as a named tuple).
I'm interested in hearing arguments against this claim, but I'm convinced that
out parameters should be replaced with named tuples, except in cases of backwards compatibility with pre-existing libraries.
Another option is to return enum with reasons such as:
public enum DeleteItemFailReasons
// other reasons
I like enums more than most of my coworkers, but I don't think this is a good reason to use them. It creates an additional maintenance task, for little or not benefit.
You're expecting the controller (or the requester) to be able to translate your enum into an error message, which means that the controller/requester must be aware of all possible errors. Furthermore, any changed/added error will require updates to the controller/requester, which means you're not segregating your responsibilities correctly.
Somewhat counterntuitively, if you were to store your error messages in a shared dictionary (as opposed to hardcoding them when you return an error), which can make sense if you use the same error message in multiple places, I would advocate using an enum as a key to that message dictionary.
But this second example (enum as a dictionary key) is different, as it remains internal to the business layer. Any changed/added errors would already require management (adding the message itself to the dictionary), so the added step of having to create an extra enum value does not (in my opinion) outweigh the benefit you get from intellisense autocompletion (which is the main reason why I advocate using the enum).
Throwing exceptions is also not good, because it would mean that I would have to used exceptions to follow program's flow, which is an anti-pattern.
Some of this is subjective. It's a matter of interpretation: is blocking behavior expected, or is it exceptional?
Shutting out the style argument for a moment, there is one objective difference: throwing and handling exceptions costs overhead. There is a performance cost attach to using exceptions.
This is the main reason why exceptions shouldn't be used wantonly to define program flow.
Like most things, it's not a zero-tolerance policy. It's a warning that when it is used ubiquitously, the performance hit will keep growing.
stringFoo + stringBar + stringBaz isn't going to bog down your application. But if you scale this up and e.g. generate an entire CSV file like this, the performance hit will become an actual problem.
Doing it once is a negligble issue. The issue only becomes relevant as you repeatedly throw errors during a single request. The more you do it, the bigger the problem.
Exceptions are perfectly fine to use as an exit strategy. And that's what you're trying to do here: the business logic error is a blocking error, meaning that there will only be one exception thrown and then the request ends. You're not creating any logic that relies on repeated exceptions to define an algorithm's usual path.
You could, for example, create a
BusinessException, so you can separate the catch logic. Business exceptions get shown to the user ad verbatim, whereas all other exception messages are used internally but not shown externally:
public string DeleteItem(int itemId)
return "Item deleted";
catch (Exception e)
return "An unexpected error has occurred.";
Having this method return a string is maybe not the best approach here, but I'm just showing a simplified example of the catch logic here.