[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