Using object state to control flow in procedures (antipattern)

I recently reviewed a piece of code such as the following.

class AntimatterCondenser(object):
def __init__(self):
self.__needs_to_send_report = False
def condense(self, container):
self.__purify(container)
self.__heat(container)
# ... some other actions
if self.__needs_to_send_report:
self.__send_report()
def __purify(self, container):
# ... do stuff
if contamination_found:
self.__needs_to_send_report = True
def __heat(self, container):
# ... do stuff
if too_much_heat_reached:
self.__needs_to_send_report = True
def __send_report(self):
# ...

Let me explain what it’s about. Imagine we are working on a device that condenses antimatter (it’s obviously not the real case I reviewed). We introduce a container full of antimatter and the device makes transformations so that the volume of the antimatter is reduced. There is a risk, though. During the process there could be some effects that resulted into unpredictable consequences, so if any of those effects are detected the device is able to send a report after doing the job.

In order to implement that behaviour, the condenser has a boolean property that acts as a flag. When it’s set to True, the device sends the report. It works, but it has a few problems.

If we run the condenser several times with different containers, and one of those containers enables the flag, the flag will remain enabled forever. It could be solved by initializing the flag at the beginning of the process.

def condense(self, container):
self.__needs_to_send_report = False
self.__purify(container)
self.__heat(container)
# ... some other actions
if self.__needs_to_send_report:
self.__send_report()

Anyway, the code looks somehow convoluted. Under what conditions are we sending the report? Which methods change the flag? What do I have to change if those rules change? The logic is hidden at that level of abstraction, so we have to look deeper in the private methods to find it out.

The problem I see with this kind of solutions (I’ve seen implementations such as this in quite a lot of situations) is that we are using the state of an object as if it was a global variable. The scope is limited to the object, that’s true, but it’s still global to the procedure. In fact, if we had that boolean attribute out of the object, there wouldn’t be a significative impact in the design.

I suggested to remove the flag and to make the code explicit by exposing the rules that define when the report needs to be sent or not.

def condense(self, container):
contamination_detected = self.__purify(container)
temperature_reached = self.__heat(container)
# ... some other actions
if (contamination_detected > SAFE_CONTAMINATION_LIMIT or
temperature_reached > SAFE_TEMPERATURE_LIMIT):
self.__send_report()

What would you do?

Written by

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store