[Tutor] simple arg problem

Steven D'Aprano steve at pearwood.info
Sat Jan 4 05:45:33 CET 2014


On Fri, Jan 03, 2014 at 09:56:25PM -0500, Keith Winston wrote:
> gmail is driving me crazy. Anyway, every time I run it with:
> 
> if __name__ == "__main__":
>     tarray = CandL_Array
>     tarray.populate(100)
> 
> I get an error
> 
> Traceback (most recent call last):
>   File "/home/keithwins/Dropbox/Python/CandL8.py", line 127, in <module>
>     tarray.populate(100)
> TypeError: populate() missing 1 required positional argument: 'gamecount1'
> >>>

Eryksun has already solved your immediate problem, but I'd like to point 
out that the above has a couple of code smells.

Are you familiar with the concept of a code smell? Code which smells 
wrong might not be wrong, but it's worth giving it a good hard long look 
just to be sure. Like Grandma used to say about your nappies, "If it 
stinks, change it", code smells usually suggest there's a problem with 
the code.

http://www.codinghorror.com/blog/2006/05/code-smells.html

Like parmesan and blue cheeses, or durian fruit, there are a few 
exceptions, but normally code is like food: it only smells bad when it 
has gone off. You should never write smelly code without giving it a 
good, hard look.

Anyway, back to your code... you have a class CandL_Array which 
apparently you call with no arguments. If it needed arguments, you 
wouldn't have made the error you did, which is to forget to include 
parentheses:

# No
tarray = CandL_Array  # makes tarray an alias to the class

# Yes
tarray = CandL_Array()  # makes tarray an instance of the class


If CandL_Array() needed arguments, you wouldn't have forgotten the round 
brackets, and wouldn't have got the error you did. So there's a little 
whiff of a smell right there... why does the class not take any 
arguments? That suggests that every instance it creates is exactly the 
same as every other instance. That's not necessarily wrong, but it is a 
little bit whiffy.

But then there's the next line:

tarray.populate(100)

Apparently, and I'm reading between the lines here, once you create the 
CandL_Array instance, you can't use it until you populate it. If I'm 
right, that's pretty smelly. That means you have errors like this:

tarray = CandL_Array()  # Initialise an instance.
tarray.play_game()  # or something, you don't show that part of the code


which blows up in your face because you forgot to call populate first. 
That's ugly, stinking code. Imagine if you had to write code like this:

x = float("12.345")
x.prepare()  # Make the float ready to use
y = x + 1.0  # Now we can use it!

Yuck.

Most of the time, creating an instance should do everything needed to 
prepare it for use. I suspect that your game is no exception. If you 
need to call some method to make the instance ready to use, then the 
constructor __new__ or initialiser __init__ should do so.

You don't even have to get rid of the populate method. You just need 
to change this:

class CandL_Array:
    def __init__(self):
        ...
    def populate(self, size):
        ...


to this:

class CandL_Array:
    def __init__(self, size):
        ...
        self.populate(size)
    def populate(self, size):
        ...


and change this:

tarray = CandL_Array()
tarray.populate(100)


to this:

tarray = CandL_Array(100)



-- 
Steven


More information about the Tutor mailing list