1

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?

2
  • What do you mean by "context object" here? Do you mean a globally accessible object? Surely then you are switching from mutating state in an injected object to globally accessible object? I don't see how that's an improvement.
    – David Arno
    Jun 17, 2016 at 13:53
  • I'm not familiar with the idea that you should not 'modify the state of an object inside of a function' being a part of OO. Is there a reference or specific school of OO that you are following?
    – JimmyJames
    Jun 17, 2016 at 14:48

4 Answers 4

5

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.

1
  • Thanks, good point about the name of the object implying that it's state will probably be changed often as a side effect. Jun 17, 2016 at 14:57
5

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

1
  • Good counter example. Thanks for the explanation. I realized I was probably wrong I just didn't know why Jun 17, 2016 at 14:57
0

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

2
  • if your language has any notion of function composition there is no need at all to "pollute" your "normal" functions with logging or error accumulators. with currying and partial application it becomes even easier. I recommend you look up how logging is usually done in functional languages, I think it could be quite illuminating. it's also funny you should mention singletons, since those kinds of global objects actually ARE considered anti-patterns.
    – sara
    Jun 17, 2016 at 16:30
  • i'm sorry i don't get you. What's the point with function composition? In functional language you usually print your logs into a static stream (e.g. a file) you don't pass a message accumulator as a parameter to all the function that need to log something. You don't do that in OO languages either, because it's a bad idea. Moreover you don't have a singleton in modern loggers, but a singleton factory , you create only one logger instance for each class that you are willing to log that is a good compromise compared to having an object that stores all the logs. Jun 17, 2016 at 20:40
-1

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.

1
  • There is nothing about OOP that presupposes imperative code (look at Scala for example), and there is nothing about functional programming that prohibits state. in FP, state is made explicit. If anything, it's shared global mutable state that is forbidden/frowned upon, but that's "forbidden" in OOP as well. It's also weird to claim that imperative code isn't as fundamentally rooted in mathematics as FP is. All of computer science started out as a sub-field of pure mathematics. You seem to have a lot of prejudice about what FP is.
    – sara
    Jun 18, 2016 at 9:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.