ErrorAccumulator as an OO pattern

Question

In the past couple of weeks I have seen a couple instances of people passing an ErrorAccumulator type of object into functions that aggregates errors as Strings inside of the function then after the function returns the errors are iterated over and logged. To me this feels like bad OO design because you are modifying the state of an object inside of a function as a side effect. Why not just store all state in a context object? Or better yet just log errors inside of the function?

Basically my question is ErrorAccumulator a pattern or anti-pattern?

Answer 1

Basically my question is ErrorAccumulator a pattern or anti-pattern?

With a few exceptions, a design can be good or bad depending on context (some designs are awful no matter how you use them). A pattern may make one design really good, but can be misapplied in another design to make it worse. Context matters.

In general, it is a good idea for functions not to modify their parameters. This is how mathematical functions operate, and it makes code clearer because of fewer side effects.

However: you need to look at each object and its purpose. When I see ErrorAccumulator I see an object designed to be modified on a whim. By its name alone, it tells me "I am a bucket to throw your errors in, and you can dump them out later."

Or better yet just log errors inside of the function?

There are some good reasons for not doing it this way, but there are a few issues here that are intertwined.

• Logging errors works okay if the code can recover from an error itself, and errors can simply go to a log file and not the user.
• Exceptions may be better if the code cannot recover from the error itself, but code higher up the call stack can do something useful.
• Accumulating errors in an object may be a good idea if the code can recover from errors itself, but you need to make a list of all the errors to show to the user as one unit.

Regarding that last bullet point, a good way to visualize this is a web form. Several fields need to be validated: maybe some only allow numbers, others must follow a specific pattern (e.g. match a regex). In this case, it may make sense to use something like that ErrorAccumulator to compile a list of errors, which can then be printed as one unit for display to the user.

Basically my question is ErrorAccumulator a pattern or anti-pattern?

Back to this question: both. If used correctly, it can solve a specific problem in software. If used incorrectly, it can solve the problem incorrectly and create confusion.

Answer 2

Why not just store all state in a context object?

So instead of passing state around in the object itself, where it's conveniently encapsulated, you would prefer to use a global variable?

Or better yet just log errors inside of the function?

Now your functions have taken on a logger dependency. Gathering errors in the object does not require this, because you can log the errors afterwards, when you are done calling your functions.

To me this feels like bad OO design because you are modifying the state of an object inside of a function as a side effect.

You may not like that, but it happens all the time.

Consider this example: moving money from one checking account to another. Your money does not know how to move itself, nor do your accounts. You have to enlist an agent (a teller, in the case of people, a Transfer object in the case of software) to move the money. There are very good reasons for this:

1. You can wrap the whole thing in a transaction, so that money isn't artificially created or lost (the debit occurs but the credit does not, etc).

2. You can do things like currency conversion during the transfer. A dollar has no idea how much it's worth in euros.

Further Reading
Unit of Work

Answer 3

It is a bad design pattern. Why a function called sum(int x, int y) should take a third argument and became sum(ErrorAccumulator ea, int x, int y)? It's very counterintuitive. Moreover suppose that there is a function foo() that does not have such parameter and does not log errors, than the need to log errors in that function arises: I have to modify that function signature and ALL the calls to that function in my code. And what if i don't have an instance of ErrorAccumulator to pass to foo() in that particular zone of code? I create a new instance but than all the errors logged before are not in the new instance and if i want all the errors i've to merge those two instances or , alternatively, i've to make sure that the same instance is available in all my system.

Luckly you don't have to reinvent the wheel. There are frameworks for logging such log4j that do exactly what you need in a smarter and more flexible way: they use a clever singleton factory patter so at any time you have the right instance of the logger, they provide simple mechanisms to log or not log things programatically so you can log errors, infos, messages and stuff according to your needs and they give a level of abstraction that permits you to log into string streams, files, remote servers etc... . Even if you are already logging into a string and you want to change and log into a file you can do that without changing a single line of code, just editing a conf file.

But since the ErrorAccumulator isn't just a poor design choice, it is also an invasive one, now for changing from ErrorAccumulator to another logger you probably have to refact your entire code...

Answer 4

To me this feels like bad OO design because you are modifying the state of an object inside of a function as a side effect.

Why is that bad? The term "side effect" comes from functional programming, which is afraid of state because tries to pretend that programming is a math problem, and math problems don't have state, so anything that doesn't fit the "math problem" paradigm must be an unwanted "side effect".

OO, on the other hand, is an imperative programming style. Imperative programming is a completely different paradigm that is not afraid of state. It treats programming like a recipe, rather than a math problem--you take data and change it into the desired form through a series of prescribed actions. In OO, externally-visible effects are not a "side" matter; they're the entire point. So don't worry about it.