your feedback to my first project please

[Jean-Michel Pichavant]

patrick at bierans.de wrote:
> Thanks for the feedback!
>
> I took the time reading and understanding it and to let it getting into my
> bones. And I also lost time on reading more of this freaky and interesting
> documentation and was testing a lot of different stuff with my enviroment.
>
> My current code can be seen here if you are interested:
>
> http://pastebin.com/3fz6qm9z  #AverageStack.py
> http://pastebin.com/tUSGs3gb  #TestAverageStack.py
>
> Here are my replies to your time consuming and informative replies ;)
>
>
>   
>> D'Arcy wrote: [code examples]
>>     
>
> Python has some really interesting tricks I have not seen in php.
> For example: self._data = [default] * dim  - That's nice. :)
> And allowing the getter to also set a value was a nice idea.
> I meant: def avg(self, value=None)
> Thanks for that! :)
>
> But I will keep some of my underscores for "private" attributes and methods.
> And I googled: "dim" was basic. I know too many languages and start mixing
> the keywords - shame on me. ;)
>
>
>   
>> D'Arcy and Peter wrote: [about writing testcases]
>>     
>
> Good Eye: I wrote the tests after coding - I know that this is the wrong way.
> I just wanted to start coding and after some time I decided that I shoud have
> some test cases. This is no real TDD - true. ;) I'll do that right next time.
>
>
>   
>> Peter wrote: You must be doing it [writing test cases] wrong.
>>     
>
> After thinking about it: Yupp. You are right. Having written tests to check
> "private" attributes really hurts one's pride. ;)
>
>
>   
>> Peter wrote: You need "from __future__ import division"
>>     
>
> Thanks for pointing me to it. I really would have fallen for it!
>
>
>   
>> Peter wrote: assert False, " %s, %s ?" % ("red", "yellow")
>>     
>
> Can you do that with a.avg() instead of a string like "red"?
>
>
> TIA,
> Patrick
>
>
> PS:
>
>   
>> gene wrote: [plain text emails]
>>     
>
> Thanks for pointing me to that. But it was the mistake of my webmailer. I am
> well aware of the etiquette of mailing lists. (As you can see my mail was
> wordwrapped accordingly.)
>
> But I will not blame you for full quoting my html email... *cough* ;-P  
>
> I hope I can get my webmailer to go for plain text email. The option is set
> but does not seam to apply. ?
import unittest
from AverageStack import AverageStack


- Module names should be lowercase (look at unittest). You can read 
http://www.python.org/dev/peps/pep-0008/ for details
- I quite dislike your avg method which does more than averaging, it 
also inserts a value and removes another modifying the object in place. 
It could be very confusing for someone reading your code. Fortunately, 
you have documented it, that makes it acceptable. Why don't you make avg 
only averaging. Surely you can use the add method right before.
- A very important information is mising from your documentation : the 
type of the parameters. Someone using your API must know which type is 
allowed. Replace "value" by "value (int)" for example.
- Another way to improve your documenation : read PEP 257 
http://www.python.org/dev/peps/pep-0257/ and try to follow these rules. 
For instance, don't write """Adds value to the stack" but """Add the 
given value (int) to the stack.""" (I would remove the internal pointer 
reference, since it's internal, the end user do not need to know about it).
- Since you write documentation (good habit :o) )  you may want to look 
at doc generators like epydoc http://epydoc.sourceforge.net/ and start 
familiarizing with any markup language (epydoc, reStructuredText ... 
your pick)

JM