Re: [Pyatcron-devel-list] Some comments on Brian code

[Julien Olivier]

On Tue, 2003-11-18 at 15:44, NICOLOVICI Xavier wrote:
> Brian,
> 
> I had a look at the code you've sent. Looks good, expect for some minor items.
> 
> 1. Your inheritance path is a bit wrong, and it does not follow what I've 
> draft on the class diagram (A task is derived from a schedule, which is not 
> what we want).
> 
> 2. Your setTime, setDate, set.... expect integer as parameters. We would like 
> to submit list of integers, which is easyer to handle from a GUI perspective. 
> Have a look at the class diagram updated by Matthew.
> 
> 3. You have introduced the owner name of a task. I'm not sure this will have 
> to be handled from the GenericTask class. I would suggest to inherit from 
> CronTask to create a SystemCronTask. As a reminder, I would say that Cron has 
> crontab entries for users and one crontab entry for the system. Only the 
> system crontab contains reference to process owner.
> 
> 
> Appart from that, I would like to rename the ScheduledTask, CronTask and 
> AtTask. Actually, it's a bit confusing with GenericTask and I fear hat in the 
> future we make some confusion. Here is my proposal:
> * ScheduledTask becomes Scheduler
> * CronTask becomes CronEntry
> * AtTask becomes AtEntry
> The inheritance path does not change.
> 
> Renaming that way makes things make things a bit smarter:
> - a planed task will be a Scheduler with its associated GenericTask
> - a crontab file will be a list of CronEntry with their associated GenericTask
> - a one time archive command will be a AtEntry with its ArchiverTask.
> 
> What do you think? Does it make sense? If it does, someone could update the 
> class diagram?
> 

For me, it makes sense. I attached an updated diagram. I changed the
classes' names, added the active property in the GenericTask as well as
the getActive, activate and deactivate methods. I also added a setTask
method in the Scheduler class and renamed the time properties/methods to
use plural.

-- 
Julien Olivier <address@hidden>

class-diagram.dia
Description: application/dia-diagram