Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Fwd: Re: Re[2]: [higgins-dev] IdAS Update Proposals
I happened to catch Valery on #higgins and asked if there were issues with these before committing them.  There were no issues with the new add* methods, but in terms of getting a single valued attribute, our proposals differ, and he had issues with what I was proposing to add.
Please take some time to review the problem and the two proposed solutions and express your view on which direction we should move (or propose an alternate if you wish)
 
Problem:
We say that an attribute may have multiple values, and we reflect this in the APIs with the method Iterator IProperty::getValues().  This is fine except for one thing; in a large number of cases, attribute will have only a single value -- causing consumers to have to do something this:
// assumes the user already knows this is a single-valued attr
Iterator iter = myAttribute.getValues();
IPropertyValue val = null;
if (iter.hasNext()) {
 val = ((IPropertyValue)iter).next();
}
 
//this could have been used to determine whether the attribute was a single-valued attr
boolean bIsSingleValued = myAttribute.getProperty().getMaxCardinality() == 1 ? true : false;
 
From a purist standpoint, this is fine -- we've provided an access method which is perfectly aligned with the model being presented.  >From a practical standpoint however, people might prefer a simpler method to access single-valued attributes.  Note that we're talking here about single-valued attribute in terms of the Context's model.  Not whether or not there happens to be a single value in a multi-valued attribute.
 
Assuming that we're correct in our belief that the half-dozen lines above are cumbersome, A number of proposals have been presented:
 
Proposal 1 (currently implemented)
Callers would probably want to precede this by first determining whether or not it's single-valued (see above)
Sample code:
IPropertyValue val;
if (myAttribute.getProperty().getMaxCardinality() == 1) {
  // handle single-val case
  val = myAttribute.getValue(true); // boolean doesn't really matter here since we've already determined
  // do something with val
} else {
  // handle multi-val case
  Iterator iter = myAttribute.getValues();
  while (iter.hasNext()) {
    val = ((IPropertyValue)iter).next();
    // do something with val
  }
}
 
Proposal 2
Sample code:
IPropertyValue val = MyAttribute.getValue();
if (!val.isList()) {
  // handle single-val case
  // do something with val
} else {
  // handle multi-val case
  IValueList list = (IValueList) val;
  Iterator iter = list.getValues();
  while (iter.hasNext()) {
    val = ((IPropertyValue)iter).next();
    // do something with val
  } 
}
 
Proposal 3
Sample code:
IPropertyValue val;
if (myAttribute.isSingleValued()) {
  // handle single-val case
  ISingleValuedProperty prop = (ISingleValuedProperty) myAttribute;
  val = prop.getValue();
  // do something with val
} else {
  // handle multi-val case
  Iterator iter = myAttribute.getValues();
  while (iter.hasNext()) {
    val = ((IPropertyValue)iter).next();
    // do something with val
  }
}
 
Proposal 4
Remove IProperty.getValue() and make people always iterate.
 
Please respond with your preference or alternate proposal.
 
Jim
 

>>> "Jim Sermersheim" <jimse@xxxxxxxxxx> 4/26/07 8:07 PM >>>
Javadoc for these changes is reflected here http://www.eclipse.org/higgins/org.eclipse.higgins.docs/idas/

>>> "Jim Sermersheim" <jimse@xxxxxxxxxx> 4/26/07 7:29 PM >>>
Here's the diff (which also includes the changes talked about in http://dev.eclipse.org/mhonarc/lists/higgins-dev/msg02312.html) of the addition of add*(<Instance of * interface>) methods.
 
I don't think there was any contention on either change, so I plan to check these both in tomorrow a.m. unless I hear otherwise.
 
Jim

>>> "Jim Sermersheim" <jimse@xxxxxxxxxx> 4/26/07 7:25 PM >>>
<fwding: sorry, I'm used to pressing reply to sender for higgins messages, which works except in the case of messages that come from Valery for some reason. So I keep replying privately to him on accident>

>>> Jim Sermersheim 4/26/07 9:18 AM >>>
I agree.  I noted this at http://wiki.eclipse.org/index.php/IdAS_Update_Proposals_Distillation#Specifying_updates #2, but I didn't include it at this point as I viewed it as not absolutely necessary. In other words, the only way I could see getting the update refactoring done in a timely way was to remove anything potentially contentious.
 
I'm happy to add this useful convenience method if no one has issues with it.  The only drawback I see is that it adds one more method to implement -- but I think it's worth it.
 
Jim
 

>>> Valery Kokhan <vkokhan@xxxxxxxxxxxxxx> 4/26/07 9:04 AM >>>
Jim,

I wonder why there is no methods like
IAttribute IDigitalSubject.addAttribute(IAttribute attr);
which you proposed earlier.

To my mind, such methods are useful when we need to copy some
attribute(s) from one digital subject to another.

--
Thanks,

Valery

Thursday, April 26, 2007, 8:41:26 AM, you wrote:

>
> 
> Here's yet another diff containing the proposed changes.  This time
> I've taken a first stab at implementing these methods in the Basic* classes.
> 
> 
> 
> I think I'll go ahead and check in tomorrow morning.

>  



_______________________________________________
higgins-dev mailing list
higgins-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/higgins-dev

Back to the top