How do I improve this code?

Question

0.00/5 (No votes)

See more:

So I have an API:

Java

@PutMapping("/{id}")
    public ResponseEntity putAccount(@PathVariable Integer id,
                                        @RequestBody AccountRequest accountRequest)
{
    AccountDto result=accountService.updateAccount(id, accountRequest);
    if (result!=null)
        return ResponseEntity.ok(result);
    else
        return ResponseEntity.ok("No such account in the DB");
}

And the implementation of the service:

public AccountDto updateAccount(Integer id, AccountRequest accountRequest)
{
    Account oldAccount = accountRepository.findById(id).orElse(null);
    if (oldAccount!=null)
    {
    UpdateNonNullFieldsUtil.updateNonNullFields(oldAccount,accountRequest);
        return AccountMapper.INSTANCE.toDto(accountRepository.save(oldAccount));
    }
    return null;
}

How do I handle errors, and do you have any suggestions for improvement if I use it in a real project?

What I have tried:

I tried private final String MESSAGE = "..." for the API response but I wonder if this is good enough for a real project.

Posted 4-Oct-24 18:00pm

Hung Le Nguyen

Updated 4-Oct-24 20:58pm

OriginalGriff

v2

Add a Solution

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Andre Oosthuizen · Accepted Answer · 2024-10-05T02:53:00

Your first step would be to learn more about error trapping and how to handle it, as a basic explanation see - Java Exceptions - Try...Catch [^]

You can now modify both your controller and service methods using the following code -

In your controller you need to return different HTTP status codes [^] codes based on your situation, the basic ones will be -
200 OK for successful updates...
404 Not Found when the account doesn't exist...
400 Bad Request for invalid input...
500 Internal Server Error for unexpected exceptions...

In your service method I have added null checks for both the id and 'accountRequest' parameters.
Instead of returning null when the account is not found, you can now throw an 'IllegalArgumentException'.
I have removed the null check and else block, as it will now throw an exception if the account is not found.

Java

//In your controller class...
@PutMapping("/{id}")
public ResponseEntity<?> putAccount(@PathVariable Integer id,
                                    @RequestBody AccountRequest accountRequest) {
    try {
        AccountDto result = accountService.updateAccount(id, accountRequest);
        if (result != null) {
            return ResponseEntity.ok(result);
        } else {
            return ResponseEntity.status(HttpStatus.NOT_FOUND)
                .body("No such account found with ID: " + id);
        }
    } catch (IllegalArgumentException e) {
        return ResponseEntity.status(HttpStatus.BAD_REQUEST)
            .body("Invalid input: " + e.getMessage());
    } catch (Exception e) {
        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
            .body("An error occurred while updating the account: " + e.getMessage());
    }
}

//In your service class...
public AccountDto updateAccount(Integer id, AccountRequest accountRequest) {
    if (id == null) {
        throw new IllegalArgumentException("Account ID cannot be null");
    }
    if (accountRequest == null) {
        throw new IllegalArgumentException("Account request cannot be null");
    }
    
    Account oldAccount = accountRepository.findById(id)
        .orElseThrow(() -> new IllegalArgumentException("No account found with ID: " + id));
    
    UpdateNonNullFieldsUtil.updateNonNullFields(oldAccount, accountRequest);
    return AccountMapper.INSTANCE.toDto(accountRepository.save(oldAccount));
}