How do I decouple a class from its attributes when methods of the attributes need to modify the state of the owning class? Alternatively, how can I redesign the architecture so that this isn't a problem?
This question is a bit abstract, but I keep coming across this problem time and time again. I spend a lot of time designing my code base so that my classes are "high cohesion and low coupling", but then as the code evolves over time, they end up becoming more closely coupled.
I'll give the latest example I have been working on. I have a Crane class with 3 Axis that can be moved. The original design had only 1 class, Crane
, and the business logic for moving each axis was repeated in the move_x
, move_y
, and move_z
methods of the Crane
. This was code duplication, so this functionality was encapsulated inside an Axis class, and then the crane class was composed of 3 of these, and now simply delegated the move_x
, move_y
, and move_z
methods to the appropriate Axis. i.e:
import asyncio
class CraneAxis:
def __init__(self):
self.__position: float = 0.0
@property
def position(self):
return self.__position
@position.setter
def position(self, value: float):
self.__position = value
print(f'new position: {value}')
async def move(self, target_position: float):
dt=0.1
crawl_velocity=0.5
if target_position > self.position:
while self.position < target_position:
self.position += crawl_velocity * dt
await asyncio.sleep(dt)
else:
while self.position > target_position:
self.position -= crawl_velocity * dt
await asyncio.sleep(dt)
class Crane:
def __init__(self):
self.x_axis = CraneAxis()
self.y_axis = CraneAxis()
self.z_axis = CraneAxis()
async def move_x(self, target_position: float):
await self.x_axis.move(target_position)
async def move_y(self, target_position: float):
await self.y_axis.move(target_position)
async def move_z(self, target_position: float):
await self.z_axis.move(target_position)
In this example, the Axis is decoupled from the Crane. I can test the Axis in isolation (i.e I do not need a Crane object to test the Axis), and changes to the Crane do not affect the Axis at all. The dependency is in one direction, the Crane depends on the Axis, but the Axis does not depend on the Crane.
We then decide that the Crane needs a status attribute that represents if any axis is currently in motion, and also it needs a way to cancel any movement.
The status attribute and the state that signals that an abort has been requested both belong to the crane, but they will need to be modified by the method belonging to the axis. I originally passed these in as parameters to the Axis move()
method: i.e:
async def move(self, target_position: float, status: CraneStatus, mode: CraneMode):
CraneStatus
and CraneMode
are enums. This is slightly more coupled to the Crane, but still pretty decoupled as it could be used by anything as long as you pass it a CraneStatus
and a CraneMode
. i.e it does not care about any implementation details, it just needs these simple things and it uses them directly.
But as I am using python these attributes would be immutable within the move()
method, and if I tried to change the value from within the function, I would instead just create a new instance. So instead I passed the owning crane into the Axis move()
method. i.e:
import asyncio
from enum import IntEnum
class CraneStatus(IntEnum):
BUSY=0,
READY=1
class CraneMode(IntEnum):
READY=0,
MOVE=1,
ABORT=2
class CraneAxis:
def __init__(self):
self.__position: float = 0.0
@property
def position(self):
return self.__position
@position.setter
def position(self, value: float):
self.__position = value
print(f'new position: {value}')
async def move(self, target_position: float, owner: 'Crane'):
if owner.crane_status == CraneStatus.BUSY:
return
owner.crane_status = CraneStatus.BUSY
owner.crane_mode = CraneMode.MOVE
dt=0.1
crawl_velocity=0.5
if target_position > self.position:
while (self.position < target_position
and owner.crane_mode != CraneMode.ABORT):
self.position += crawl_velocity * dt
await asyncio.sleep(dt)
else:
while (self.position > target_position
and owner.crane_mode != CraneMode.ABORT):
self.position -= crawl_velocity * dt
await asyncio.sleep(dt)
owner.crane_status = CraneStatus.READY
owner.crane_mode = CraneMode.READY
class Crane:
def __init__(self):
self.__crane_status = CraneStatus.READY
self.__crane_mode = CraneMode.READY
self.x_axis = CraneAxis()
self.y_axis = CraneAxis()
self.z_axis = CraneAxis()
@property
def crane_status(self):
return self.__crane_status
@crane_status.setter
def crane_status(self, value: CraneStatus):
self.__crane_status = value
print(f'new crane status: {value}')
@property
def crane_mode(self):
return self.__crane_mode
@crane_mode.setter
def crane_mode(self, value: CraneMode):
self.__crane_mode = value
print(f'new crane mode: {value}')
async def move_x(self, target_position: float):
await self.x_axis.move(target_position, self)
async def move_y(self, target_position: float):
await self.y_axis.move(target_position, self)
async def move_z(self, target_position: float):
await self.z_axis.move(target_position, self)
def abort(self):
self.crane_mode = CraneMode.ABORT
At this point, I noticed something that keeps happening in my codebases. Often when I use composition, I end up passing an owner
parameter into the methods (or the constructor) of the object that is being used to compose the owning class. i.e in this example, the Axis now needs to be passed the Crane object. I have lost the decoupling. I now need a Crane object to test the Axis, and the Axis is sensitive to changes in the Crane. The dependency is in two directions, the Crane depends on the Axis, and the Axis depends on the Crane.
Perhaps this isn't a problem at all, and having these classes coupled is no big deal. But I have been taught that tight coupling is bad. Is this the case?
If I did want to decouple the Axis and the Crane, what would be the best way to go about this?
Thanks!
edit: Just to make it clear, this is a question about code quality and maintainability, and not about getting something to work. The code in the above examples behave exactly how I want them to, I just want to make the implementation better.
Also, I am aware that python is dynamically typed, and I have used it in a statically typed way in the above examples, but I also have this problem in statically typed languages and would like a solution that I can use in any language. Also, for this project, we have decided to type check the code base with MyPy
, and are trying to use things in a more strongly typed way to try and avoid bugs.