mercredi 6 janvier 2021

How to decouple a class from it's attributes when the methods of the attributes need to modify state of the owning class?

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.

Aucun commentaire:

Enregistrer un commentaire