问题描述:

I'm new to OOP so I need help to understand if this is the correct way to handle it.

I have a 3-Layer web project (+ DTOs) and I'm trying to understand wich is the best way to return "errors" from the Business Objects to the Presentation Layer.

In particular now I'm facing this creating a user. Let's say I want to create the the user in the db upon website registration and I need to tell the actual user if the username or email have already been taken (this is just an example).

The ASP.NET Membership.CreateUser() method for example pass a MembershipCreateStatus object byRef, so the method is used to return an Enum (wich resides in another namespace...) to pass the status of the attempt and this could be DuplicateEmail, DuplicateUserName, and so on.

I implemented another way based on exceptions but I'd like your opinion on it. In the BLL manager class that creates the user I created a nested Excpetion class and nested Enums for error type this way:

Public Class UserManager

Public Enum ErrorType

DatabaseError = 1

UserExists = 2

EmailExists = 3

End Enum

Public Class ManagerException

Inherits Exception

Public Property ErrorType As ErrorType

Public Property SuggestedUserName As String

Public Property SuggestedEmail As String

Public Sub New()

MyBase.New()

End Sub

Public Sub New(message As String)

MyBase.New(message)

End Sub

Public Sub New(message As String, inner As Exception)

MyBase.New(message, inner)

End Sub

End Class

Public Function CreateUserLogin(user As EvaVwUserLogin) As Guid

If user Is Nothing Then

Throw New ApplicationException("No user suppied")

End If

If String.IsNullOrWhiteSpace(user.Password) OrElse String.IsNullOrWhiteSpace(user.UserName) Then

Throw New ApplicationException("Password or username missing")

End If

If CheckDuplicateUserName() Then

Dim ex As New ManagerException("Username exists")

ex.ErrorType = ErrorType.UserExists

ex.SuggestedUserName = "this username is free"

Throw ex

End If

End Function

End Class

Then in the UI layer (aspx code behind) I call the manager and check the exception like this:

 Dim userManager As New UserManager

Try

userManager.CreateUserLogin("test", "test")

Catch ex As UserManager.ManagerException

If ex.ErrorType = userManager.ErrorType.UserExists Then

Dim suggestedUsername = ex.SuggestedUserName

' Display error message and suggested user name

End If

End Try

Is this a correct approach, considering that both the exception and the enums are very specific to this manager, so that if I go down this route, each manager will have its nested ManagerException with related enums?

Thanks in advance for your opinions as always.


Following up the "custom code" scenario kindly suggested by Brian and Cyborg (I marked his answer just because it's more complete Brian maybe other users are interested in MS advice) do you think that nesting the custom return object and relative enums in the manager class would be a good idea?

Since those enums will be strictly related to this manager class (and each managar class in the BLL would have his own) do you think I could still use them as the status code passed?

EDIT: I refactored this way... do you think it's ok?

Public Class UserManager

Public Enum ErrorType

DatabaseError = 1

UserExists = 2

EmailExists = 3

End Enum

Public Class ErrorData

Public Property ErrorType As ErrorType

Public Property SuggestedUserName As String

Public Property SuggestedEmail As String

End Class

Public Function CreateUserLogin(username As String, password As String, ByRef errorData As ErrorData) As Guid

Dim user As New EvaVwUserLogin

user.UserName = username

user.Password = password

Return CreateUserLogin(user, errorData)

End Function

Public Function CreateUserLogin(user As EvaVwUserLogin, ByRef errorData As ErrorData) As Guid

If user Is Nothing Then

Throw New ApplicationException("No user object")

End If

If String.IsNullOrWhiteSpace(user.Password) OrElse String.IsNullOrWhiteSpace(user.UserName) Then

Throw New ApplicationException("Missing password or username")

End If

Dim hashedPassword As String

hashedPassword = Crypto.HashPassword(user.Password)

If UserExists(user) Then

errorData.ErrorType = ErrorType.UserExists

errorData.SuggestedUserName = "this username is free"

Return Nothing

End If

.....

End Function

End Class

And in the Presentation Layer:

 Dim userManager As New UserManager

Dim managerError As New UserManager.ErrorData

Dim userId As Guid

userId = userManager.CreateUserLogin("test", "test", managerError)

If userId = Nothing Then

If managerError.ErrorType = userManager.ErrorType.UserExists Then

Dim suggestedUserName = managerError.SuggestedUserName

' Do something with suggested user name

End If

End If

网友答案:

(This answer is just to provide support for @BrianMains suggestion to use return codes rather than exceptions. The relevant documentation was too large to fit in a comment.)

From the MSDN documentation on exceptions:

A significant amount of system resources and execution time are used when you throw or handle an exception. Throw exceptions only to handle truly extraordinary conditions, not to handle predictable events or flow control. For example, your application can reasonably throw an exception if a method argument is invalid because you expect to call your method with valid parameters. An invalid method argument means something extraordinary has occurred. Conversely, do not throw an exception if user input is invalid because you can expect users to occasionally enter invalid data. In such a case, provide a retry mechanism so users can enter valid input.

Throw exceptions only for extraordinary conditions, then catch exceptions in a general purpose exception handler that applies to the majority of your application, not a handler that applies to a specific exception. The rationale for this approach is that most errors can be handled by validation and error handling code in proximity to the error; no exception needs to be thrown or caught. The general purpose exception handler catches truly unexpected exceptions thrown anywhere in the application.

In addition, do not throw an exception when a return code is sufficient; do not convert a return code to an exception; and do not routinely catch an exception, ignore it, then continue processing.

http://msdn.microsoft.com/en-us/library/system.exception.aspx

(Highlighting mine)

网友答案:

For that, I would follow what the Membership API does. Return an enum from your business component, which the view can use to check the response. So you return an enum with values like you mention to the ASP.NET page, and check the type there. You can do what you are doing, but from what I see I don't think it's necessary; plus, throwing exceptions are expensive.

To clarify what I mean by enum is return it from the caller of the business component, not via an exception.

相关阅读:
Top