patternjavaMinor
Does this method contain multiple implementations?
Viewed 0 times
thisimplementationsmethodcontainmultipledoes
Problem
My teammate and I have a small discussion about the responsibility of a method. He thinks next method has two responsibilities and I think that only one.
I think that the responsibilities of validating the string "username" falls on the "validate" method of my mAccountValidator collaborator. Depending of the result returned by my collaborator, I change the state of the view and finally, in order to finish the process or not, I return the state of validation. This result is managed by the next method:
What do you think about my solution? Do you think that is it enough clean or how I could improve it?
Thank a lot for your time and help.
private boolean validateUsername(String username) {
AccountValidationResult validationResult =
mAccountValidator.validate(username, new int[]{AccountValidation.VALIDATION_REQUIRED});
if (!validationResult.isSuccessful()) {
mView.setUsernameModeError(validationResult.getMessage());
} else {
mView.setUsernameModeStandard();
}
return validationResult.isSuccessful();
}I think that the responsibilities of validating the string "username" falls on the "validate" method of my mAccountValidator collaborator. Depending of the result returned by my collaborator, I change the state of the view and finally, in order to finish the process or not, I return the state of validation. This result is managed by the next method:
public void doLogin(UserCredentialModel userCredential) {
if (!validateUsername(userCredential.getUsername())) {
return;
}
if (!validatePassword(userCredential.getPassword())) {
return;
}
mDoLogin.setUserCredential(userCredential);
mDoLogin.execute();
}What do you think about my solution? Do you think that is it enough clean or how I could improve it?
Thank a lot for your time and help.
Solution
Seemed fine at first, then I took another look...
The validation function displays the error message?
I'm sorry, but that's indeed a violation of SRP. You'd be better off returning a
What's wrong is that your function names do not reflect this. You call
The validation function displays the error message?
I'm sorry, but that's indeed a violation of SRP. You'd be better off returning a
ValidationResult which you could use. You already HAVE this validation result... so in a way, you've already done what I said.What's wrong is that your function names do not reflect this. You call
validateUsername, but it also displays errors. Rather than changing the validation method to return a ValidationResult (which would imply taking out validateUsername and doing everything in doLogin, you should change the function name instead.Context
StackExchange Code Review Q#135266, answer score: 6
Revisions (0)
No revisions yet.