지정된 케이스를 처리 할 수없는 경우 switch 문에서 예외 발생
MVC 앱의 시스템에서 사용자의 비밀번호를 변경하는 기능이 있다고 가정 해 보겠습니다. :
public JsonResult ChangePassword
(string username, string currentPassword, string newPassword)
{
switch (this.membershipService.ValidateLogin(username, currentPassword))
{
case UserValidationResult.BasUsername:
case UserValidationResult.BadPassword:
// abort: return JsonResult with localized error message
// for invalid username/pass combo.
case UserValidationResult.TrialExpired
// abort: return JsonResult with localized error message
// that user cannot login because their trial period has expired
case UserValidationResult.Success:
break;
}
// NOW change password now that user is validated
}
membershipService.ValidateLogin()
UserValidationResult
다음과 같이 정의 된 열거 형을 반환합니다 .
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
Success
}
방어적인 프로그래머이기 때문에 ChangePassword()
인식 할 수없는 UserValidationResult
값 이있는 경우 예외를 throw하도록 위의 메서드를 변경합니다 ValidateLogin()
.
public JsonResult ChangePassword
(string username, string currentPassword, string newPassword)
{
switch (this.membershipService.ValidateLogin(username, currentPassword))
{
case UserValidationResult.BasUsername:
case UserValidationResult.BadPassword:
// abort: return JsonResult with localized error message
// for invalid username/pass combo.
case UserValidationResult.TrialExpired
// abort: return JsonResult with localized error message
// that user cannot login because their trial period has expired
case UserValidationResult.Success:
break;
default:
throw new NotImplementedException
("Unrecognized UserValidationResult value.");
// or NotSupportedException()
break;
}
// Change password now that user is validated
}
저는 항상 모범 사례 위의 마지막 스 니펫과 같은 패턴을 고려했습니다. 예를 들어, 한 개발자가 사용자가 로그인을 시도 할 때이 또는 그 사업상의 이유로 비즈니스에 먼저 연락해야한다는 요구 사항을 받으면 어떻게 될까요? 따라서 UserValidationResult
의 정의는 다음과 같이 업데이트됩니다.
enum UserValidationResult
{
BadUsername,
BadPassword,
TrialExpired,
ContactUs,
Success
}
The developer changes the body of the ValidateLogin()
method to return the new enum value (UserValidationResult.ContactUs
) when applicable, but forgets to update ChangePassword()
. Without the exception in the switch, the user is still allowed to change their password when their login attempt shouldn't even be validated in the first place!
Is it just me, or is this default: throw new Exception()
a good idea? I saw it some years ago and always (after groking it) assume it to be a best practice.
I always throw an exception in this case. Consider using InvalidEnumArgumentException
, which gives richer information in this situation.
With what you have it is fine although the break statement after it will never be hit because execution of that thread will cease when an exception is thrown and unhandled.
I've used this practice before for specific instances when the enumeration lives "far" from it's use, but in cases where the enumeration is really close and dedicated to specific feature it seems like a little bit much.
In all likelihood, I suspect an debug assertion failure is probably more appropriate.
http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx
Note the second code sample...
I always like to do exactly what you have, although I usually throw an ArgumentException
if it's an argument that was passed in, but I kind of like NotImplementedException
better since the error is likely that a new case statement should be added rather than the caller should change the argument to a supported one.
I never add a break after a throw statement contained in a switch statement. Not the least of the concerns is the annoying "unreachable code detected" warning. So yes, it is a good idea.
I think approaches like this can be very useful (for example, see Erroneously empty code paths). It's even better if you can have that default-throw be compiled away in released code, like an assert. That way you have the extra defensiveness during development and testing, but don't incur the extra code overhead when releasing.
You do state that you're being very defensive and I might almost say this is overboard. Isn't the other developer going to test their code? Surely, when they do the simplest of tests they'll see that the user can still log in - so, then they'll realize what they need to fix. What you're doing isn't horrible or wrong, but if you're spending a lot of your time doing it, it might just be too much.
For a web application I would prefer a default that generates a result with an error message that asks the user to contact an adminstrator and logs an error rather than throws an exception that might result in something less meaningful to the user. Since I can anticipate, in this case, that the return value might be something other than what I expect I would not consider this truly exceptional behavior.
Also, your use case that results in the additional enum value shouldn't introduce an error into your particular method. My expectation would be that the use case only applies on login -- which would happen before the ChangePassword method could legally be called, presumably, since you'd want to make sure that the person was logged in before changing their password -- you should never actually see the ContactUs value as a return from the password validation. The use case would specify that if a ContactUs result is return, that authentication fails until the condition resulting in the result is cleared. If it were otherwise, I'd expect that you'd have requirements for how other parts of the system should react under that condition and you'd write tests that would force you to change the code in this method to conform to those tests and address the new return value.
Validating runtime constraints is usually a good thing, so yes, best practice, helps with 'fail fast' principle and you are halting (or logging) when detecting an error condition rather than continuing silently. There are cases where that is not true, but given switch statements I suspect we do not see them very often.
To elaborate, switch statements can always be replaced by if/else blocks. Looking at it from this perspective, we see the throw vs do not throw in a default switch case is about equivalent to this example:
if( i == 0 ) {
} else { // i > 0
}
vs
if( i == 0 ) {
} else if ( i > 0 ) {
} else {
// i < 0 is now an enforced error
throw new Illegal(...)
}
The second example is usually considered better since you get a failure when an error constraint is violated rather than continuing under a potentially incorrect assumption.
If instead we want:
if( i == 0 ) {
} else { // i != 0
// we can handle both positve or negative, just not zero
}
Then, I suspect in practice that last case will likely appear as an if/else statement. Because of that the switch statement so often resembles the second if/else block, that the throws is usually a best practice.
A few more considerations are worthwhile: - consider a polymorphic approach or an enum method to replace the switch statement altogether, eg: Methods inside enum in C# - if throwing is the best, as noted in other answers prefer to use a specific exception type to avoid boiler plate code, eg: InvalidEnumArgumentException
'development' 카테고리의 다른 글
WiX를 사용하여 Windows 서비스를 설치하고 시작하는 방법 (0) | 2020.12.12 |
---|---|
SetupSet ()는 더 이상 사용되지 않습니다. (0) | 2020.12.12 |
PID와 TID의 차이점 (0) | 2020.12.12 |
Node.js / 서버 측 자바 스크립트에서 .NET DLL 사용 (0) | 2020.12.12 |
Notepad ++로 파일 저장을위한 기본 인코딩 변경 (0) | 2020.12.12 |