Вопрос

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
Это было полезно?

Решение 2

(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.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top