Code Separation in Service Layer and Controller

Question

I have written code for reset password and other parts of my application, I want to separate service layer from dao and controller; my controller code is:

@RequestMapping(value = "", method = RequestMethod.PUT)
public ResponseModel resetPassword(@Valid @RequestBody AuthenticationRequestModel authenticationRequestModel, HttpServletRequest request) {
    String ip = WebUtils.getClientIp(request);
    Optional<SecuritySMS> securitySMS = securitySMSService.getLastValidSMS(authenticationRequestModel.getMobile());
    if (!securitySMS.isPresent()) {
        return new ResponseModel(messages.get("sms.security.expired"), ResponseModel.ResponseStatus.ERROR);
    }
    SecuritySMS sms = securitySMS.get();
    if (!sms.isConfirmed()) {
        return new ResponseModel(messages.get("sms.security.expired"), ResponseModel.ResponseStatus.ERROR);
    } else if (!sms.getIp().equalsIgnoreCase(ip)) {
        return new ResponseModel(messages.get("sms.security.ip.changed"), ResponseModel.ResponseStatus.ERROR);
    }
    Optional<User> user = userService.findByMobile(sms.getMobile());
    if (!user.isPresent()) {
        return new ResponseModel(messages.get("sms.reset.user.nonexistent"), ResponseModel.ResponseStatus.ERROR);
    }

    userService.updatePassword(user.get(), authenticationRequestModel.getPassword());
    return authHelper.loginWithHttpResponse(authenticationRequestModel);
}

I am confusing about moving code to the service layer; is above approach correct or must some code (for example below code) should move to the service layer? if I move this code to the service layer, how can I get response back? Boolean (not acceptable because I want to return correct message to user) ,string or exception?;

String ip = WebUtils.getClientIp(request);
Optional<SecuritySMS> securitySMS = securitySMSService.getLastValidSMS(authenticationRequestModel.getMobile());
if (!securitySMS.isPresent()) {
    return new ResponseModel(messages.get("sms.security.expired"), ResponseModel.ResponseStatus.ERROR);
}
SecuritySMS sms = securitySMS.get();
if (!sms.isConfirmed()) {
    return new ResponseModel(messages.get("sms.security.expired"), ResponseModel.ResponseStatus.ERROR);
} else if (!sms.getIp().equalsIgnoreCase(ip)) {
    return new ResponseModel(messages.get("sms.security.ip.changed"), ResponseModel.ResponseStatus.ERROR);
}
Optional<User> user = userService.findByMobile(sms.getMobile());
if (!user.isPresent()) {
    return new ResponseModel(messages.get("sms.reset.user.nonexistent"), ResponseModel.ResponseStatus.ERROR);
}

Answer 1

MVS is just a simplification and it's not a perfect approach. That is why it raises so many questions similar to this one.

• How much logic should go into a controller?
• Should a model contain any logic?
• Should a view contain logic?

Questions like these are very hard to answer, but it is for the controller to validate inputs and a redirect to views when needed. Your code does seem a lot like domain logic, which should go in the service later, but it clearly is easier to managed in your controller. For domain logic (aka business logic, business rules, and domain knowledge), we can explain it as the the logic that makes business-critical decisions.

So, true, you are taking decisions in your Controller, but managing these decisions in the service complicates the solution. So, I would just leave it there, anyways in the end, no one but you and your group will contribute to this code, so if you are comfortable with it, is fine.

But, if you were to move your code, I would recommend the use of exceptions. Services, in my opinion, should only return desired models or resources. Is better to handle errors using exceptions.