Re: [Tutor] Lights game

[Kent Johnson]

Nice work overall! A few comments inline...
Kent
Ismael Garrido wrote:
Hello list.
I'd really appreciate any comments, particulary regarding style 
corrections. I'm a newbie...

Thanks!
Ismael
import random
import tkMessageBox
from Tkinter import *
lights is only needed by GUI(), it is a helper class. It would be better style to construct Luces() 
in GUI.__init__() and save it as an attribute. Then GUI stands by itself - it doesn't rely on a 
global variable or any outside setup. (You will have to change all accesses of lights to use 
self.lights)

class GUI:
   def __init__(self):
         self.lights = Luces()
       self._crearGUI()
   def _crearGUI(self):
       self.root = Tk()
       self.root.title("Lights")
       self.botones = [[0,0,0],[0,0,0],[0,0,0]]
       for i in range(3):
           for j in range(3):
               action = lambda n=i, m=j: self._accion(n,m)
               self.botones[i][j] = Button(self.root, height=5, 
width=10, command=action)
               self.botones[i][j].grid(row=i, column=j, padx= 4, pady=4)

       self._updateBotones()
       self.root.mainloop()
   def _accion(self, i, j):
       self._click(i, j)
       try:
           self._checkWin()
       except ValueError:
This is a little strange.
I would have _checkWin() return True or False instead of throwing an exception 
when the user wins!
Then you would write
        if self._checkWin():
           tkMessageBox.showinfo(title = "You won!", message = "You 
rock, kid!")

           if True == tkMessageBox.askyesno(title="What shall we do?", 
message="Play another?"):
               self.root.destroy()
               lights.__init__() ## Is this... too ugly? Is there a 
beter way?
               self.__init__()
You could use
  lights._hacerTablero()
  self._crearGUI()
I agree it is little wierd to call __init__ from another function. Better to have a separate method 
that does exactly what you want. Then if the requirements of __init__ change you will still be doing 
the right thing here.

Instead of destroying the window, why not just update it?
           if True == tkMessageBox.askyesno(title="What shall we do?", message="Play 
another?"):
               lights._hacerTablero()
               self._updateBotones()
           else:
               self.root.destroy()

           else:
               self.root.destroy()
   def _updateBotones(self):
       for i in range(3):
           for j in range(3):
               if lights.tablero[i][j] == True:
                   self.botones[i][j].configure(bg="red")
               else:
                   self.botones[i][j].configure(bg="grey")
   def _click(self, i, j):
       lights.click(i,j)
       self._updateBotones()
   def _checkWin(self):
       conteo = 0
       for i in range(3):
           for j in range(3):
               if lights.tablero[i][j] == True: conteo +=1
       if conteo == 9:
           print "GANO"
           raise ValueError
                 return
  if conteo == 9:
    print "GANO"
    return True
  else:
    return False
or maybe even just
  return conteo == 9
class Luces:
   def __init__(self):
       self.tablero = [[0,0,0],[0,0,0],[0,0,0]]
       self._hacerTablero()
   def _hacerTablero(self):
       for i in range(3):
           for j in range(3):
               self.tablero[i][j] = not random.randint(0,1)
   def _cambiarCelda(self, i, j):
       self.tablero[i][j] = not self.tablero[i][j]
If you make _cambiarCelda() check for out-of-range indices then you could get rid of 
_descartarNegativos() and clean up click() considerably.

   def _cambiarCelda(self, i, j):
       if not (0<=i<3 and 0<=j<3):
           return
       self.tablero[i][j] = not self.tablero[i][j]
   def click(self, i,j):
       self._cambiarCelda(i,j)
       self._cambiarCelda(i-1, j)
       self._cambiarCelda(i+1, j)
       self._cambiarCelda(i, j-1)
       self._cambiarCelda(i, j+1)
or if you prefer:
   def click(self, i,j):
       for ii, jj in [ (i,j), (i-1, j), (i+1, j), (i, j-1), (i, j+1) ]:
           self._cambiarCelda(ii, jj)
   def _descartarNegativos(self, n):
       if n < 0:
           raise IndexError
       return n
   def click(self, i,j):
       self._cambiarCelda(i,j)
       try:
           self._cambiarCelda(self._descartarNegativos(i-1), j)
       except IndexError:
           pass
       try:
           self._cambiarCelda(self._descartarNegativos(i+1), j)
       except IndexError:
           pass
       try:
           self._cambiarCelda(i, self._descartarNegativos(j-1))
       except IndexError:
           pass
       try:
           self._cambiarCelda(i, self._descartarNegativos(j+1))
       except IndexError:
           pass
if __name__ == '__main__':
   lights = Luces()
Not needed if you make the change I suggest at the top.
   lightsGUI = GUI()

  _______________________________________________
Tutor maillist  -  Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor
_______________________________________________
Tutor maillist  -  Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor