1

I have a service that performs a series of actions based on user information (name, address, etc), one of the problems is that the User entity comes from a legacy third party API (as a repository) that did not enforce a strong validation on data, so some users may have invalid information, such as blank or incomplete name, address, etc and thus cannot proceed to complete the service's flow.

the API provider updated its API and provided an endpoint to reset specific user information, but I'm only able to know which users have invalid information when they try to use the service and an exception is thrown, so my approach to solve this problem was to add this "rollback logic" inside a catch block and send it to the API to be reset, here are some of my concerns:

1 - I'm not sure that this is an elegant solution, I'm handling errors and the "rollback logic" in a catch block, this is something I have never done before and, I honestly don't know if this is a use case for the catch block, is it okay? is there a better way to handle this?

2 - There can be multiple different exceptions for each field (InvalidNameException, InvalidAddressException, InvalidDogexception...), I have to find a way to group exceptions together in order to send the specific group of fields that need to be reset, otherwise I'm going to send multiple different requests, making the user unnecessarily repeat the service's flow several times.

for example:

A user has its name and address invalid, but the InvalidNameException was thrown first, so only the 'name' field is going to be sent to the API to be reset, but I need both (name and address)

I know that the use of exceptions is subjective to the context, and I believe that its use in this context is "correct-ish" because not having valid user information is an exceptional case, but when it comes to doing specific stuff based on specific exceptions I find it kind of hard to deal with exceptions, but I do not know a better way to handle errors...

here's an example of the current structure:

The service:

class DoSomethingService {

    public constructor(
        private userRepositoryInterface: UserRepositoryInterface,
        private registrationManagerInterface: RegistrationManagerInterface
    ) { }

    public execute(input: ServiceInput): void {

        try {
            const user = this.userRepositoryInterface.findById(input.userId);
            //... bussiness actions
        } catch (error) {

            //only the first exception is going to be caught
            // use strategy perhaps?
            let fieldsToReset = [];
            if (error instanceof InvalidAddressException) {
                fieldsToReset.push('address');
            } else if (error instanceof InvalidNameException) {
                fieldsToReset.push('name')
            }

            this.registrationManagerInterface.resetUserInformation(input.userId, fieldsToReset)
        }
    }
}

The User entity and the Address value object with example validation:

class User {
    public constructor(
        private name: FullName,
        private address: Address
    ) { }
}

class Address {
    public constructor(private street: string) {
        if (street.length <= 0) {
            throw new InvalidAddressException('Invalid Street')
        }
    }
}
5
  • If that's your only way of getting the information I don't see that you can do this elegantly, you'll have to bug the user over each field and they'll hate you. Much better would be to retrieve all the fields and check their validity. And what's an InvalidDogException, a cat? :) May 2 at 5:34
  • After resetting/correcting one field, is it possible that you just automatically retry the service's operation for that User? There is no point in bothering the client/human user with having to do the retry if it can also be done automatically. May 2 at 6:36
  • Does this answer your question? Is it good practice to use try catch for data validation
    – gnat
    May 2 at 6:54
  • 1
    Who owns the User and Address classes? The question reads as if the third party API is throwing exceptions, but is that really the case?
    – Rik D
    May 2 at 7:31
  • @RikD the User entity and its Value Objects are constructed from the third party API response, it's a repository May 2 at 11:50

1 Answer 1

4

You are asking the wrong question.

If exceptions are the way provided by an API for detecting some invalid state, you obviously must use them, that's not "good" or "bad", but simply inevitable.

However, what looks like the road to a big-ball-of-mud hell is mixing up the business logic of the service with the validation and correction of the data, where validation and correction could (and most probably should) be done beforehand as separate steps. This is where "separation of concerns" could help.

Better organize the code in three separate steps:

  1. Loop over the fields you want to validate, and collect the "fields to reset", by using a try-catch inside the loop.

  2. If the resulting list of "fields to reset" is not empty, then initiate the reset (ideally with one request). If necessary, go back to step 1 until the list of "fields to reset" is empty

  3. After the data was cleaned up, start with the main business logic.

Of course, steps 2 and 3 (as well as the initial request in step 1) may require error handling, hence additional try-catch blocks for specificially this purpose.

In short: using a catch block for a rollback mechanism is not a problem. Putting different kinds of error handling for different process steps as well as a rollback mechanism into one large catch block is what makes it a problem. Better separate these things.

3
  • I think i choose a bad title for this question haha, my fault.. I agree that this is not ideal because this is a problem for the specific repository (from the third party API) and is mixing business with technical stuff, but the problem is that I cannot iterate over all the fields because the User entity and its Value Objects are created from the API response, the validation is "atomic", either the User is created with valid value objects or it's not because the first is going to throw an exception May 2 at 11:54
  • @ThiagoDias: would you mind to give a short outline of what happens inside //... bussiness actions to provoke the exception?
    – Doc Brown
    May 2 at 15:07
  • sure, i'm going to edit the question and the title May 2 at 16:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.