Is it bad practice for an object to catch its own exception and store the message in a property?

Question

Let's say I have a list of objects of the same type. I want to iterate over that list of objects and execute a "dangerous" method on each of them. If an exception occurs in that method, is it bad practice to have the method itself catch the exception and set an error property on the object?

Here's a quick example where the Start() method of the Car catches its own exception and stores it in its "Problem" property:

Sub Main()

    // Build cars.
    Dim ford As New Car("ford", "1988")
    Dim chevy As New Car("chevy", "1992")
    Dim honda As New Car("honda", "2002")

    // Create a list of cars.
    Dim carList As New List(Of Car)
    carList.Add(ford)
    carList.Add(chevy)
    carList.Add(honda)

    // Start all of the cars.
    For Each c As Car In carList
        // I don't want to stop processing if an exception occurs.
        // And I don't care to report any errors now.
        c.Start()
    Next

    // other stuff...

    // Now report errors.
    Dim cr As New CarReport
    cr.ReportProblems(carList)

End Sub

The class that reports any problems:

Public Class CarReport

    Public Sub ReportProblems(ByVal c As List(Of Car))
        // Report any problems.
        For Each c In carList
            If Not String.IsNullOrEmpty(c.Problem) Then
                Console.WriteLine(c.Problem)
            End If
        Next
    End Sub

End Class

The simple Car class:

Public Class Car

    Private _make As String
    Private _year As String
    Private _problem As String = String.Empty

    Public Sub New(ByVal make As String, ByVal year As String)
        _make = make
        _year = year
    End Sub

    Public Sub Start()
        Try
            // Do something that could throw an exception.
        Catch ex As Exception
            _problem = "Car would not start."
        End Try
    End Sub

    Public ReadOnly Property Problem() As String
        Get
            Return _problem
        End Get
    End Property

End Class

EDIT: What if the caller (i.e., Sub Main() above) caught the exception and then set the Problem property on the Car object? That seems cleaner. What I really need to accomplish is to keep the error text with the particular Car object so it can be passed along to another system for reporting.

Note: The car scenario is a trivial example to illustrate a scenario, I didn't put much thought into the class design.

Answer 1

I would say absolutely, because this is only a provision for a single call. What happens if you make other call on the object and it throws an exception as well? The property value is overridden and then you aren't really sure what the state of the object is.

You are better off capturing exception information in a try/catch block and then working on it separately. Generally speaking, unless you have designed the method of an object to not modify the state of the object, I would consider an exception thrown on a call to a method/property of the object to render the object unusable, as the state of the object is indeterminate at that point.

Answer 2

I think so for a number of reasons:

(1) It violates the Fail Fast principle by delaying the reporting of the problem.

(2) It isn't a very modular design. The car and carReport classes are too tightly coupled.

(3) It requires the caller to have special knowledge of the implementation of main() to know how to deal with errors making it more likely that the client code will neglect to handle the error condition either out of ignorance of the error handling mechanism or laziness.

(4) If you have a situation where multiple errors can occur like this, it has a certain smell that maybe the operation should be broken down into smaller units of work.

(5) As mentioned in another answer. The car.problem and car.start methods are too tightly coupled. For example, what happens if you call car.start twice and it only fails once? Do you retain the first problem? How does the caller know how to interpret the car.problem property in this situation. Overall it just seems like bad encapsulation.

Answer 3

This is not a bad practice, it is a design pattern that is used in ADO.Net.

Each row in a dataset has a property called HasErrors. This is useful in various validation scenarios.

Here is an example by Beth Massi: "Displaying Data Validation Messages in WPF"

EDIT: @casperOne: Thanks for your comment. Yes, the HasError behavior violates the ideal of encapsulation -- once the object has an error some outside action has to collect/use that information and then destroy/reset the object.

But it is a nice compromise of simplicity. A "correct" implementation involving wrappers would be complex and expensive, therefore impractical.

This is not only an appeal to authority, it is an appeal to "the damn thing works!" ;)

Answer 4

In that special way you use it I think it is bad practice yes. The main problem I see is that the class using the object is not informed of problem that occured, it has to check itself, although it is responsible for error handling. If the car itself does the error handling, nothing stands against it as it can make sure the object state remains consistent.

Although this patern is used in ADO.Net as 'Tom A' said, I think it is used in another context. As clearly statet that is a validation context. No real error/exception occured there, but it might! There you have the possibility to correct the data that might crash, in the context used here the error already occured.

Answer 5

I think the distinction needs to be drawn between 'Data Validation' and 'Program Errors'.
In the data validation scenario:

If the validation is being driven in full or part by exceptions, catching the exception and applying it to a property, either through the ADO.NET HasErrors, or IDataErrorInfo, or whatever means you need is a good thing. There are many arguments over whether using exceptions for validtion is good programming practice or not and one that I've seen good code on both sides of the fence for.

However, if an exception comes around that isn't related to your validation (NullReferenceException, or MemoryException) such that your object can no longer function correctly because its state has been undefined, then the exception needs to raised for the reasons mentioned in JohnFx's answer.

Answer 6

var errors = new string[carList.Length];
for(var i = 0; i < carList.Length; i += 1) {
    try {
        carList[i].Start();
    } catch(Exception ex) {
        errors[i] = ex.Message;
    }
}