Re: [Contacts] couple of comments

On 7.1.2010, at 10.50, ext Max Froumentin wrote:

> 1. I think that examples would be more instructve if they included  
> some
> code showing how to access data from callback objects, instead of  
> "// do
> something with resulting contact objects"
>
>
> 2. "Device implements DeviceContacts;"
>
> Why did you choose "implements" over inheritance or PrototypeRoot? I
> don't know which to choose myself.
>
> 3. "ContactError::CONTACT_INVALID_ERROR."
>
> Replace :: with . ?
>
> 4. "One or more contexts to associated with the given value"
>
> Typo


Some additional comments against the current version:

4.4.1

* The categories attribute could be renamed to tags. Categories are  
not tags but tags can be also categories, right? Also categories name  
may suggest it is a bounded set.

* "The following constants are defined for use in the types attribute  
for email addresses:
'home', 'work', 'mobile', 'personal', 'business'." The mobile is not  
consistent with the others so it could be dropped. Otherwise we could  
argue that we should add 'desktop' and 'laptop' as well and the  
combinations 'home-mobile' and 'work-laptop' etc.

* impps attribute name could be more descriptive to be easier to  
remember and type correctly. However, I don't have a good suggestion  
at the moment :)

* impps and phones attributes: "The first object in this sequence is  
inferred to be the user's preferred/default instant messaging and  
presence protocol address.", perhaps we should add a simple example on  
how to change the defaults w/o removing the existing entries (there  
are many ways -- some more complex than others -- to do array  
manipulation)? E.g.:

// get the myContact object somehow
// ...
// change myContact defaults w/o removing the existing entries
myContact.impps.unshift({ types: ['home'], value: 'sip:new-default@example.org 
' });
myContact.emails.unshift({ types: ['work'], value:  
'john.doe@example.org' });

4.6.1

"The way to group the first enumerable ContactProperties attribute  
provided in the related method. Only the first ContactProperties  
property provided can be grouped." ECMA-262 section 12.6.4 "The for-in  
Statement" says: "[t]he mechanics and order of enumerating the  
properties [...] is not specified", thus we cannot rely on the order  
of the ContactProperties properties as the Note suggests. One solution  
would be to replace the group attribute with an orderBy attribute  
which defines the property name according to which to sort. The sort  
attribute could also be renamed to sortOrder and its value could be  
case-insensitive.

-Anssi

Received on Friday, 8 January 2010 18:09:43 UTC