298099 - Suggestions for calIRecurrenceRule for infinite recurrence

Status: NEW

(Calendar :: Internal Components, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+
Build Identifier: trunk

Suggestions for calIRecurrenceRule for infinite recurrence

calIRecurrenceRule says:

 63   // These two are mutually exclusive; whichever is set
 64   // invalidates the other.  It's only valid to read the one
 65   // that was set; the other will throw NS_ERROR_FAILURE.  Use
 66   // isByCount to figure out whether count or endDate is valid.
 67   // Setting count to -1 or endDate to null indicates infinite
 68   // recurrence.
 69   attribute long count;
 70   attribute calIDateTime endDate;
 71 
 72   // if this isn't infinite recurrence, this flag indicates whether
 73   // it was set by count or not
 74   readonly attribute boolean isByCount;

As I read this, it is unreasonably difficult to tell whether a
rule is infinite; it requires either omnisciently knowing ahead of
time whether count or endDate was set, or some code that catches
errors if it reads the wrong one.

  var isInfinite;
  try { 
    isInfinite = (rule.count == -1);
  } catch (nsErrorFalure) {
    isInfinite = (rule.endDate == null);
  }

Two suggestions:
(0. rename recurrence.endDate to recurrence.untilDate, bug 298097)

1. isByCount should be valid regardless of whether it is infinite or
   not to tell which attribute is readable.
   Suggest dropping "if this isn't infinite recurrence" from line 72,
   and putting isByCount before count and untilDate.  Reword
   isByCount as:
     // isByCount indicates whether the count attribute is readable..
     // If not, the untilDate attribute is readable.
     readonly attribute boolean isByCount;    

2. After change 1, it still requires something like 

     var isInf = (rule.isByCount? rule.count == -1 : rule.untilDate == null);

   Which would be better implemented as an attribute so it
   could be simply read through the attribute:

     var isInf = rule.isInfinite;

   Suggest:
     // isInfinite indicates whether the rule repeats forever.
     // If not, either rule.count != -1 or rule.untilDate != null.
     readonly attribute boolean isInfinite;


Reproducible: Always

Steps to Reproduce:

Comment 1

[gekacheka]

*** Bug 328444 has been marked as a duplicate of this bug. ***

Comment 2

[Joey Minta]

calIRecurrenceRule extends calIRecurrenceItem which exposes isFinite.  This is WFM as there is already another bug on renaming endDate to untilDate.

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

calIRecurrenceRule still says that you specify infinite recurrence with endDate and count. This is stil confusing and still needs to be fixed.