-2

I have a class called 'Movable Piece'. Of course, I want every instance of this class to move. For that, I thought another class called 'Movement' would be nice, and reusable in case I need other stuff to move. Besides, I quite like how my_piece.move.up looks in the code.

The problem comes when I realize I need to dynamically try to set methods for the instance of the Movements class instantiated by the Piece, as the functions that move the piece may as well be user-defined. How can I achieve this? I think the code will clarify what I want to do.

class MovablePiece(Piece):
    class Movements:
        def __init__(self, piece, movement_functions=None):
            if movement_functions is None:
                self.__default_movements(piece)
            else:
                self.__set_movements(movement_functions)

        def __default_movements(self, piece):
            def up(): return piece.move(piece.surroundings[Direction.UP])
            def right(): return piece.move(piece.surroundings[Direction.RIGHT])
            def down(): return piece.move(piece.surroundings[Direction.DOWN])
            def left(): return piece.move(piece.surroundings[Direction.LEFT])
            self.__set_movements([up, right, down, left])

        def __set_movements(self, movement_functions):
            for movement_function in movement_functions:
                setattr(self, movement_function.__name__, movement_function)

    def __init__(self, letter, name, movements=None, walkable=False):
        Piece.__init__(self, letter, name, walkable)
        self.move = MovablePiece.Movements()

This, of course, won't work: setattr is trying to set a function as an attribute, which I don't think makes much sense, but you get the gist of it.

This is the error when I try to do my_piece.move.right:

Traceback (most recent call last):
  File "main.py", line 45, in <module>
    screen.show()
  File "/home/joaquin/Documents/escape/ludema/screen.py", line 12, in show
    function()
  File "main.py", line 35, in control_bruma
    mappings[action]()
  File "/home/joaquin/Documents/escape/ludema/pieces.py", line 78, in right
    def right(): return piece.move(piece.surroundings[Direction.RIGHT])
TypeError: 'Movements' object is not callable

Similar problem if I force the methods to be staticmethods (as they don't actually require 'self'):

Traceback (most recent call last):
  File "main.py", line 45, in <module>
    screen.show()
  File "/home/joaquin/Documents/escape/ludema/screen.py", line 12, in show
    function()
  File "main.py", line 35, in control_bruma
    mappings[action]()
TypeError: 'staticmethod' object is not callable
Pablo
  • 4,821
  • 12
  • 52
  • 82
joaquinlpereyra
  • 956
  • 7
  • 17
  • `setattr()` should work fine. The problem may be that the functions aren't defined to accept a `self` first argument ,so they aren't proper methods. – martineau Aug 13 '16 at 21:26
  • It doesn't. I'll attach the error given by the traceback. – joaquinlpereyra Aug 13 '16 at 21:28
  • @martineau I've added the traceback for when I try to make them staticmethods, which you take no parameters easily :) – joaquinlpereyra Aug 13 '16 at 21:32
  • `self.move = MovablePiece.Movements()` sets `self.move` to an instance of the `Movements` class, which isn't callable. You probably need to define a `__call__()` method. – martineau Aug 13 '16 at 21:33
  • @martineau Yeah, but I'm not trying to call self.move (never in the code does self.move() appears), I'm trying to calls self.move.up (or down, or left, or right). – joaquinlpereyra Aug 13 '16 at 21:35
  • 2
    @joaquinlpereyra What do you get from this rather than having a mix in movement class exactly? – Jon Clements Aug 13 '16 at 21:40
  • `return piece.move(piece.surroundings[Direction.RIGHT])` _is_ trying to call the object assigned to `self.move`. It also shows that the the `right()` staticmethod function has been called. – martineau Aug 13 '16 at 21:45
  • Upon further review, `self.move = MovablePiece.Movements()` will result in a `TypeError: __init__() missing 1 required positional argument: 'piece'`. Please [edit] your question and provide a [MCVE](https://stackoverflow.com/help/mcve). – martineau Aug 14 '16 at 14:07
  • @martineau Thank you for your help. The crucial part was your second last comment: of course piece.move was trying to call the object assigned. I thank you for your efforts! This led me to solve it. Curiusly, I didn't need to make it a staticmethod: apparently Python automagically inserts the self parameter into a function when you set it with setattr. I'll post the answer. – joaquinlpereyra Aug 14 '16 at 15:26

2 Answers2

1

IMHO you should have provided a mvce in the question that way this answer could add some additional tips, in any case, here's a working example guessing the missing bits of your code:

class Piece(object):

    def __init__(self, letter, name, walkable):
        self.letter = letter
        self.name = name
        self.walkable = walkable


class Movements:

    def __init__(self, piece, movement_functions=None):
        if movement_functions is None:
            self.__default_movements(piece)
        else:
            self.__set_movements(movement_functions)

    def __default_movements(self, piece):
        def up(): print("up")

        def right(): print("right")

        def down(): print("down")

        def left(): print("left")
        self.__set_movements([up, right, down, left])

    def __set_movements(self, movement_functions):
        for movement_function in movement_functions:
            setattr(self, movement_function.__name__, movement_function)


class MovablePiece(Piece):

    def __init__(self, letter, name, movements=None, walkable=False):
        Piece.__init__(self, letter, name, walkable)
        self.move = Movements(self)

p = MovablePiece("foo", "foo")
for direction in ["up", "right", "down", "left"]:
    getattr(p.move, direction)()

Another choice would be coding something like this:

class UpMovement(object):

    def __init__(self, piece):
        self.piece = piece
        self.name = "up"

    def move(self):
        if self.piece.walkable:
            print("up")
        else:
            print("piece not walkable to go up")


class DownMovement(object):

    def __init__(self, piece):
        self.piece = piece
        self.name = "down"

    def move(self):
        if self.piece.walkable:
            print("down")
        else:
            print("piece not walkable to go down")


class LeftMovement(object):

    def __init__(self, piece):
        self.piece = piece
        self.name = "left"

    def move(self):
        if self.piece.walkable:
            print("left")
        else:
            print("piece not walkable to go left")


class RightMovement(object):

    def __init__(self, piece):
        self.piece = piece
        self.name = "right"

    def move(self):
        if self.piece.walkable:
            print("right")
        else:
            print("piece not walkable to go right")


class Piece(object):

    def __init__(self, letter, name, walkable):
        self.letter = letter
        self.name = name
        self.walkable = walkable


class Movements(object):

    def __init__(self):
        pass


class MovablePiece(Piece):

    def __init__(self, letter, name):
        Piece.__init__(self, letter, name, True)
        movements = [
            UpMovement(self),
            DownMovement(self),
            LeftMovement(self),
            RightMovement(self)
        ]

        self.move = Movements()
        for m in movements:
            setattr(self.move, m.name, m.move)


class StaticPiece(Piece):

    def __init__(self, letter, name):
        Piece.__init__(self, letter, name, False)
        movements = [
            UpMovement(self),
            DownMovement(self),
            LeftMovement(self),
            RightMovement(self)
        ]

        self.move = Movements()
        for m in movements:
            setattr(self.move, m.name, m.move)

p1 = MovablePiece("foo1", "foo1")

for name in ["up", "down", "left", "right"]:
    getattr(p1.move, name)()

p2 = StaticPiece("foo2", "foo2")

for name in ["up", "down", "left", "right"]:
    getattr(p2.move, name)()

Of course, you could overengineer the thing abstracting classes here and there, making the class design much better and applying SOLID design principles. In any case, the question was basically how to attach dynamically stuff to Pieces, so here's a possible solution :)

Community
  • 1
  • 1
BPL
  • 9,632
  • 9
  • 59
  • 117
  • Thank you for your response! Unfortunately, the first option requires a lot of lines from the end programmer and could lead easily to confusion (I'm programming a library, so the end user is a programmer too): I would really like to attach the creation of the Piece with the creation of the movements. The second options creates a lot of unnecessary classes. I've finally solved it, I'll answer my own question if you are interested in seeing how I've done it. – joaquinlpereyra Aug 14 '16 at 15:30
  • @joaquinlpereyra Glad you found your answer. I haven't provided many more solutions to the problem cos the question wasn't specifying any more requirements and it was quite open, as you can see I've guessed a lot. Next time just provide a [mcve](http://stackoverflow.com/help/mcve) with more constraints and you'll get better answers. Good luck. – BPL Aug 14 '16 at 15:37
0

This is how I finally ended up doing it. I'm sorry this example is not reproducible, but there are just too many classes involved in the mix and I think it would just tamper with readability and comprehension for this precise problem. You can nevertheless peep the code at github.

Notably, I didn't have to force the functions to be static even when they take no parameters. Apparently Python does that for you, somehow.

class MovablePiece(Piece):

    class Movements:
        """A simple interface to represent the movements of the MovablePiece.
        """
        def __init__(self, piece, movement_functions=None):
            if movement_functions is None:
                self.__default_movements(piece)
            else:
                self.__set_movements(movement_functions)

        def __default_movements(self, piece):
            def up(): return piece.move_to_tile(piece.surroundings[Direction.UP])
            def right(): return piece.move_to_tile(piece.surroundings[Direction.RIGHT])
            def down(): return piece.move_to_tile(piece.surroundings[Direction.DOWN])
            def left(): return piece.move_to_tile(piece.surroundings[Direction.LEFT])
            self.__set_movements([up, right, down, left])

        def __set_movements(self, movement_functions):
            for movement_function in movement_functions:
                setattr(self, movement_function.__name__, movement_function)

    def __init__(self, letter, name, movements=None, walkable=False):
        Piece.__init__(self, letter, name, walkable)
        self.move = MovablePiece.Movements(self)

    def _unsafe_move_to_tile(self, tile):
        """Move the object in a certain direction, if it can:
        That means: unlink the piece from its current tile and link it
        to the new tile; unless there's a piece in the destiny tile already.

        Return True if could move there, False is possition was already
        ocuppied.

        Can raise a PieceIsNotOnATileError if the piece hasn't been put on a
        map prior to moving or a PieceIsNotOnThisBoardError if the piece
        you're trying to move has an associated tile in another board, not
        the one where the destinity tile is.
        """
        if not self.home_tile:
            raise PieceIsNotOnATileError
        if self.home_tile.board is not tile.board:
            raise PieceIsNotOnThisBoardError

        if tile.piece is not None:
            tile.piece.on_touch_do(touching_piece=self)
            if not tile.piece.walkable:
                return False

        self.home_tile.piece = None
        tile.piece = self
        return True

    def move_to_tile(self, tile):
        if tile:
            try:
                return self._unsafe_move_to_tile(tile)
            except (PieceIsNotOnATileError, PieceIsNotOnThisBoardError):
                return False
        else:
            return False
joaquinlpereyra
  • 956
  • 7
  • 17
  • FWIW: You didn't need to make the functions staticmethods because you're adding them to a class instance — `self` — not a class. See the accepted answer to question [_Adding a Method to an Existing Object Instance_](http://stackoverflow.com/questions/972/adding-a-method-to-an-existing-object-instance) for more information. – martineau Aug 17 '16 at 14:03