[Tutor] Code review, plase

Steven D'Aprano steve at pearwood.info
Wed Sep 8 00:20:21 CEST 2010


On Wed, 8 Sep 2010 06:39:27 am Alex wrote:
> Hi all.
>
> Could someone review my code? It's the first time I develop a
> reusable module and I would like to have some feedback.
> If you think it's good enough I will package it for pypi.
>
> I put the code on pastebin: http://pastebin.com/Tz367gAM

Let's start with some little things... you talk about "the Unix CronTab" 
command, but there's no such thing, at least on Linux:

[steve at sylar ~]$ CronTab
bash: CronTab: command not found

It would be very unusual for a Unix command to be written in CamelCase. 
The command is actually "crontab".

In the _remove_crontab method, you say "*All* the information contained 
in the crontab file are permanently lost", but that's grammatically 
incorrect -- "information" is a mass noun, i.e. neither singular nor 
plural. So in standard English, you would say "all the information in 
the file is permanently lost", not "are lost", in the same way that you 
would say "the sugar is on the table" or "the grass is green".

http://en.wikipedia.org/wiki/Mass_noun

You overuse leading-underscore method names. Use them only for private 
methods. You use them for public methods, and even give an example of 
how to remove the entire file:

>>> #Remove the entire crontab file
>>> crontab._remove_crontab()

When you're telling people to use this command, that's a good sign that 
it is public not private!

In PRESETS, what's "mothly" mean? *wink*

What happens if I do this?

ct1 = micron.CronTab()
ct2 = micron.CronTab()
ct1.add_job('daily', 'echo "BOOM!"')
ct2.add_job('daily', 'echo "No BOOM today"')

Do they fight? What happens?



-- 
Steven D'Aprano


More information about the Tutor mailing list