[Tutor] simple arg problem
spir
denis.spir at gmail.com
Sat Jan 4 11:11:36 CET 2014
On 01/04/2014 05:45 AM, Steven D'Aprano wrote:
> 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)
Waow! that was problem analysis.
Denis
More information about the Tutor
mailing list