[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