# Script for Python Hanoi Tower OOP

Is my Python Script to play Hanoi Tower respectful of OOP spirit?

I have difficulties to create meaningful classes so I started an easy project. Here is one class at the plateau level and then a class to handle towers.

How can it be improved (on the OOP level and on script level)?

I want then a GUI with TKinter, a third class GUI would be suited ?

Thank you

import numpy as np
NB_DISKS = 3

class Plateau:
"""Class for the whole plateau"""
def __init__(self):
self.init_plateau()

def init_plateau(self):
"""List of towers,first tower full"""
self.nb_disks = NB_DISKS
self.towers = [Tower(self.nb_disks), Tower(0), Tower(0)]

def motion(self, tower_from, tower_to):
"""Motion from one tower to another one with checking"""
from_tower = self.towers[tower_from-1]
to_tower = self.towers[tower_to-1]

if from_tower.get_last_disk()>to_tower.get_last_disk():  #Receive 0 if to_tower is empty
disk = from_tower.get_last_disk()
from_tower.delete_disk()

self.check_victory()
self.print_towers()
else:
print('Last disk number from origin has to be bigger than reception one')

def check_victory(self):
"""Check if last tower has an ordered list or not"""
last_tower = self.towers[2].get_whole_tower()
if len(last_tower) == self.nb_disks:
diff_last_tower = np.diff(last_tower)
if np.unique(diff_last_tower) == 1:
print('victory:')
self.print_towers()
print('new plateau:')
self.init_plateau()

def print_towers(self):
"""Print the towers"""
for elt in self.towers:
elt.print_tower()
print('\n')

class Tower:
"""Class for a tower"""
def __init__(self,nb_disks):
"""Creation of a tower"""
if nb_disks !=0:
self.tower = list(range(1, nb_disks+1))
else:
self.tower = []

def get_last_disk(self):
"""Return last element of the tower"""
if self.tower == []:
return 0
else:
return self.tower[-1]

def delete_disk(self):
"""Delete last disk of the tower"""
self.tower.pop()

"""Add a disk to the top of the tower"""
self.tower.append(disk)

def get_whole_tower(self):
"""Return the complete tower"""
return(self.tower)

def print_tower(self):
"""Print the element of the tower"""
print(self.tower)

if __name__ == '__main__':
game = Plateau()
game.motion(1,3)
game.motion(1,3)   # Try to move a big disk on a small one
game.motion(1,2)
game.motion(3,2)
game.motion(1,3)
game.motion(2,1)
game.motion(2,3)
game.motion(1,3)


From the OOP/typing level, you might want to consider having Disk be a type. If it doesn't need any methods, you could just make it a subtype of int:

from typing import NewType

Disk = NewType('Disk', int)


1. Since you always use get_last_disk and delete_disk together, why not combine them? The thing you really want is a pop_top_disk operation that removes and returns the last disk (that's already how you're implementing delete_disk anyway).

2. add_disk could implement the "no larger disk on a smaller one" rule.

3. get_whole_tower seems bad because it's exposing the internal data structure of the tower; this violates your entire OOP abstraction. Note also that in general you should distinguish between "private" and "public" members of your classes.

4. Rather than making NB_DISKS a global and having Plateau initialize itself from that, Plateau's initializer should take the number of disks as a parameter.

5. The Pythonic way of making a "print" function is to implement the magic function __repr__ to return a string.

6. The int values of your disks should be reversed so that bigger numbers represent bigger disks.

7. This is more an English thing than a Python thing, but the verb form of "motion" is "move" so it would be better to name your method move. :)

Here's the final code I came up with after implementing the above suggestions:

from typing import NewType

Disk = NewType('Disk', int)

class WrongOrder(Exception):
def __init__(self, top: int, bottom: int) -> None:
super().__init__("Can't put a disk of size %d on a disk of size %d" % (top, bottom))

class Plateau:
"""Class for the whole plateau"""
def __init__(self, nb_disks: int) -> None:
self._init_plateau(nb_disks)

def _init_plateau(self, nb_disks: int) -> None:
self._towers = [Tower(nb_disks), Tower(0), Tower(0)]

def move(self, from_tower: int, to_tower: int) -> None:
"""Move from one tower to another (towers specified as an int from 1-3).
Prints an error if the move is invalid."""
# Convert from 1-index to 0-index.
from_tower -= 1
to_tower -= 1
# Make the move, print exception if it fails.
try:
disk = self._towers[from_tower].pop_top_disk()
try:
except WrongOrder:
# don't drop the disk!  Put it back where we got it and reraise.
raise
except Exception as e:
print('Move failed: ', str(e))
else:
self._check_victory()
print(self)

def _check_victory(self) -> None:
"""Check if all disks have moved to the last tower (victory condition).
If the player has achieved victory, reset the game.
"""
if sum(tower.height for tower in self._towers) == self._towers[2].height:
print('victory:')
print(self)
print('new plateau:')
self._init_plateau(self._towers[2].height)

def __repr__(self) -> str:
"""Print the towers"""
return "\n".join([repr(tower) for tower in self._towers]) + "\n"

class Tower:
"""Class for a tower"""
def __init__(self, nb_disks: int) -> None:
"""Creation of a tower"""
self._tower = [Disk(size) for size in range(1, nb_disks + 1)]
self._tower.reverse()  # order the stack from largest to smallest

def pop_top_disk(self) -> Disk:
"""Remove and return the top disk on this tower."""
return self._tower.pop()

def add_disk(self, disk: Disk) -> None:
"""Add a disk to the top of the tower.
Raises WrongOrder if the disk being added is too big.
"""
if len(self._tower) and self._tower[-1] < disk:
raise WrongOrder(disk, self._tower[-1])
self._tower.append(disk)

@property
def height(self) -> int:
"""Number of disks in the tower."""
return len(self._tower)

def __repr__(self) -> str:
"""Print the elements of the tower"""
return repr(self._tower)

if __name__ == '__main__':
game = Plateau(3)
game.move(1,3)
game.move(1,3)   # Try to move a big disk on a small one
game.move(1,2)
game.move(3,2)
game.move(1,3)
game.move(2,1)
game.move(2,3)
game.move(1,3)

• This is mostly good. I may add an answer of my own, but I wanted to react to your point #1: In OP's code, they did not always use get_last_disk and delete_disk together, because they weren't treating illegal moves as exceptional. Between your nested-try pattern and what OP wrote, I think I'd prefer the original! – ShapeOfMatter Dec 24 '19 at 20:02
• Thanks, you provided me nice advices on POO and Python. I will look more for @Property, repr and exception handling. NewType seems also to be a nice feature – Kerdiorp Dec 25 '19 at 10:16

The "spirit" of Object Oriented Programming is two-fold:

• To give us an enforceable metaphor for reasoning about our code. ("objects" as objects)
• To give us heuristics about how to compartmentalize out code. (encapsulation)

The purist perspective of OOP can be a bit verbose, and in python it's not usually necessary.

I want then a GUI with TKinter, a third class GUI would be suited ?

Try thinking about it like this: An object method will have both a method that it's called on and a context in which it gets called. If you build a class to encapsulate your GUI, where will it's methods get called from?

One of the principal things objects do is manage state. If a function needs to update state, that suggests that the function and the state could be encapsulated together in an object. If the function just needs to read a state, and that state (data) could be passed in as an argument by the calling context, then the function can be a static method or a global function. If a class only has static methods, then you don't need a class at all. That's a good thing: the less state you're managing the less opportunity to mess it up.

How can it be improved (on the OOP level and on script level)?

Sam Stafford's points #4 and #5 are good, as is the suggestion to have Disk as a new type.

1. You could also neglect to declare a proper class for the Towers by just having Tower = NewType('Tower', List(Disk)).
2. If you're thinking of the normal input and output as a user-interface, then you probably shouldn't be printing (or reading input) from inside class methods. That said, logging is a fine thing to do, and print is a low-effort way to do it.
3. Plateau.motion() does too many things. Checking for victory should certainly go outside in the calling context. I would suggest that validating the user-input also doesn't belong in there.
4. Similarly, Plateau.check_victory() shouldn't set up the new game, and init_plateau should get inlined into Plateau.__init__(). When you start a new game just build a new Plateau.

Taken to the extreme, you could have a static class representing the state of the game, and a function to start a new game, and then you'd repeatedly call a function (GameState, PlayerMove)->GameState. At that point you'd be breaking past traditional imperative OOP into a more "functional" style.

• Ok thanks, I will focus on writing smaller methods – Kerdiorp Dec 25 '19 at 10:20