[Tutor] Need a solution.

[Alan Gauld]

"David" <david at abbottdavid.com> wrote 

>> class GuessError(Exception): pass
>> class GuessedNumber:
>>     def __init__(self,tries=None,limits=None):...
>>     def generate(self, limits=None):...
>>     def __cmp__(self, aNumber):
>>           if self.count >= self.tries: raise GuessError
> Thanks always for the feedback, i came up with this, not sure if it is 
> much better but I am learning as I prod along :)

The big problem from an OO point of view is that you are not 
making the objects do any work. You are extracting the data 
from inside them and you are doing all the work in your main 
function. In OOP we should be trying to make the objects do 
everything and the main function just does some high level 
coordination.

If you make your GuessedNumber class have comparison 
methods you can avoid much of that. And if you put the counting 
code inside the comparison that could save a lot of effort in main 
too. Remember that in theory you shold not know what data is 
inside the object. You should only interact with objects via their 
published operations.

> #!/usr/bin/python
> from random import randrange
> from sys import exit
> 
> class GuessedNumber:
>     def __init__(self, attempts=None):
>         self.attempts = attempts
>         self.number = randrange(1,99)

Any time you have a class that just has an __init__ it means 
its not doing anything. And that's a bad sign. Classes are there 
to *do* things not just store data. We can use a tuple or 
dictionary to do that.

> class Counter:
>     def __init__(self):
>         self.value = 0
>     def step(self):
>         self.value += 1
>     def current(self):
>         return self.value

Whilst this is OK,, because it does something, its a lot of 
code to wrap an integer. I personally wouldn't bother. 
But at least it is doing things :-)

> def play():
>     c = Counter()
>     guessit = GuessedNumber(attempts=5)
>     target_number = guessit.number
>     attempts = guessit.attempts

See, here is the problem, you create an object then 
immediately extract all the data and then just ignore 
the object. You might as well just assign the values 
to variables


>     guess = int(raw_input('Guess-> '))
>     c.step()
>     while c.current() < attempts:

So why not 

         while c.current() < guessit.attempts

use the object, thats why its there

>         try:
>             if guess == target_number:

Whereas this could have been 

               if guess == guessIt

>                 print "Well Done"
>                 play_again()
>             elif guess < target_number:

and         elif guess < guessit

>                 print 'Higher ... '
>                 guess = int(raw_input('Guess Again-> '))
>                 c.step()
>             elif guess > target_number:
>                 print 'Lower ... '
>                 guess = int(raw_input('Guess Again-> '))
>                 c.step()
> 
>         except ValueError:
>             print 'You must enter a number'
>             pass
> 
>     print 'Too many attempts, the number was', target_number
>     play_again()
> def play_again():
>     answer = raw_input('Do you want to try again? y/n ')
>     answer = answer.lower()
>     if answer == 'y':
>         play()
>     else:
>         exit()

This recursive approach will work most of the time but 
remember that Python does limit recursion to 1000 levels 
so if your player was very keen you could run out of levels. 
A normal loop would be safer.

HTH,

Alan G.