[Tutor] instance method call issue

[Steven D'Aprano]

On 14/10/12 23:34, Osemeka Osuagwu wrote:

> In the code below, I can't seem to get around calling an instance
> method (__reality_check()) from within a class method (update_grid()),

Of course you can't :)

Since the class method has no access to the instance, it cannot call
instance methods, since it doesn't know which instance to use!

Instead, update_grid needs to be an instance method. Of course it
does! It needs to update a specific grid, right? That will be the
instance.


> I'm not even too sure that update_grid() should have been a class
> method in the first place.


Probably not. Static and class methods are rarely used, since they have
no access to the instance that owns them.

Class methods are generally used for alternate constructor functions, e.g.
dict.fromkeys. Since it makes no difference whether you call fromkeys
from the class (dict) or from an instance, it is made a classmethod.

Static methods, well, 99 times out of a hundred if you think you want to
use a static method, you're probably better off making it a module level
function.

By my count, there are over 26000 instance methods in the Python standard
library, 202 class methods, and only 84 static methods. So they are rare.
I've been using Python for about 15 years, and I think I could count the
number of times I've used staticmethod on the fingers of one hand.



> Please, I need help with this and any other advice on my coding style,
> where I could've been more efficient etc.
>
>
> class Grid:
>          '''Grid(rows = 26, columns = 26) ->  Provides a world object
> for the cells in Game of Life'''
>          from time import sleep
>          from os import system
>          from sys import platform

Move the imports outside of the class. They should go at the start of
the module.

As written, your class has attributes Grid.sleep, Grid.system, etc. Since
these attributes have nothing to do with Grids, they don't belong in the
Grid class. Move them out.

Classes are not catch-all collections of unrelated methods. That's
what modules are for :)




>          #class data:
>          __rows, __array, os_type = [''], [], platform

A word of advise: don't waste your time with double underscore "private"
attributes. They're no more private than single underscore (private by
convention only), and the name mangling can cause difficulty when
debugging.

There are good reasons for using double underscores like this, but you
should consider them a bit more advanced.


>          @staticmethod
>          def display_grid(generation, os_type):
>                  try:
>                          (lambda x:Grid.system('cls') if 'win' in
> os_type else Grid.system('clear'))(1)


That's wrong! Your display_grid method uses the class (Grid), so it
shouldn't be a static method.

Move the imports to the module level, and display_grid as a regular
function:

def display_grid(generation, array):
     clear_screen()
     print 'Now in Generation %d' % generation
     for line in array:
         print line

def clear_screen(platform=sys.platform()):
     # The default here is set once, and defaults to your platform.
     if 'win' in platform:
         command = 'cls'
     else:
         command = 'clear'
     os.system(command)


Or leave display_grid as an instance method:


class Grid:
     def display_grid(self, generation):
     clear_screen()
     print 'Now in Generation %d' % generation
     for line in self._array:
         print line


Either way, clear_screen has nothing to do with grids, so it should
not be a method of Grid.



>                  except:
>                          print 'Cannot display Grid'

Never use a bare except like that. Bare except should *only* be used
at the interactive interpreter, for lazy people who want to avoid
typing. It should never be used non-interactively, it's simply poor
practice.

The problem with bare excepts is that they cover up too much. Your aim
is to write code that works. If it has a bug, you need to *see* the bug
so you can identify it and then fix it. Bare excepts make bugs
invisible.



>          @staticmethod
>          def extend_grid(thickness = 4, force_extend = False):
>                  '''extend_grid([thickness][, force_extend]) -->
> Extends the edges of the grid by 4 units in
> all directions if the boundary of the simulation comes close to the
> edge. Extends anyway if force_extend option is True'''

Why is this a static method?

Which grid is this operating on? It should either take a grid as an
argument, or extend_grid should be an instance method that operates
on itself:

# either this top-level function
def extend_grid(grid, thickness=4, force=False):
     # do stuff to the grid to extend it
     # and then return it
     return grid


# or make this a regular instance method
class Grid:
     def extend_grid(self, thickness=4, force=False):
         # do stuff to self to extend it
         # no need to return anything



>          @classmethod
>          def update_grid(cls, generations, speed):


Why is this a class method?


>                  '''Runs the 'Life' World simulation for the specified
> number of generations at the given speed'''
>                  #control = input
>                  #while
>                  for times in range(generations + 1):

This contains an off-by-one bug. If you pass generations=100 as argument,
it will run 101 times instead.

Instead, if you want your generations to count from 1:

for times in range(1, generations + 1):
     ...


>                          cls.extend_grid()
>                          cls.__reality_check(Grid, 'all')

Having made this a class method, why have you hard-coded a reference to
the Grid class in it's body?


>                          cls.sleep(speed)
>                  return

There's very rarely any need for a bare return like that. Just leave it
out, Python automatically returns when it drops out the bottom of the
function.


[...]
>          def __reality_check(self, num = 'all'):
[...]
>                          print 'Wrong argument for reality check'

Have you learned about raising exceptions yet? I see above you use a
try...except loop, so you should know about them.

Don't just print a message when your method receives an invalid argument,
raise an exception:

     raise ValueError("expected num='all' or an integer, got %r instead" % num)


>          def edit_cell(self, cells, state = '##'):
>                  '''edit_cell(cells[, state]) -->  Where cells is a list
> of tuples containing coordinates of the cells to edit'''
>                  cells = cells
>                  for eachcell in cells:
>                          Grid.__array[eachcell[0]][eachcell[1]] = state
>                  return

This is an abuse of the class. You have instances, but they all share state.
Why? Sharing state is okay if you have a reason for wanting to share state,
but here there doesn't appear to be any reason for it.

As you have written this, you cannot have two Grids. Actually you can, but
since they both share the same state, you cannot have two DIFFERENT Grids.


My advice is, forget about class and static methods completely until you
are a bit more experienced with object-oriented design.



-- 
Steven