Re: Patch for review: timeout audio alarms

[rms39 columbia edu (Russell Steinthal)]

Miguel de Icaza writes:
>
>Hello Russell,
>
>   The timeout alarm patch seems fine.  I would like to suggest that
>   you add some error checking for making sure that the user does not
>   enter invalid values for the timeout values and to make sure that
>   the information we fetch from the configuration file is also
>   within a reasonable range.

Invalid values are simple: timeouts must be positive integers.  The
problem, however, is figuring out what the "reasonable range" is: for
example, I might want to timeout my alarm after one minute, while
someone else wants to wait two hours.  (Both are "features" of
different alarm clocks I have in my room. I'm one of those people who
needs two to get up in the morning. :))

So would 1 hour be a reasonable maximum?  1 day?  (The latter is in
line with the rest of the code which seems to handle alarms one day at
a time.)

>   In general, if this is not done in the rest of the code, it should
>   be done.

Agreed, although the same problems arise.  Where we have a discrete
enumeration of possible values (for example alarm types), checking is
simple, if a bit time-consuming (for the programmer that has to write
all the if-else blocks or whatever).  It's the "reasonable" limits on
times, dates, which get into problems.  Note that I'm not saying that
there are any good examples of those in the code at the moment (I
haven't looked or thought too hard about it) but that it's a potential
issue.

As an aside, it might be wise to consider a more organized system for
storing these configuration variables in memory- as of now we have a
mess of global variables, which is most likely not the best
programming practice.  (And I notice it caused Eskil some problems
with the conduit stuff when I added a new variable but didn't know to
add it to the conduit capplet.)

A structure (or several structures) would reduce the namespace
pollution problem, but I don't know if it would help Eskil's problem
(which I still don't fully understand even after getting an
explanation from him).  It would certainly be cleaner, though,
particularly if it mirrored the gnome_config or properties page
structure.

-Russell