Re[3]: Fwd: Re: Re[2]: [higgins-dev] IdAS Update Proposals

[Valery Kokhan <vkokhan@xxxxxxxxxxxxxx>]

I think we all agree that this entire topic is just about user
convenience. We want to be nice to users and we have a good will to
help them with this:

IProperty prop = null;
IPropertyValue val = null;

if (prop.getPropertyModel().getMaxCardinality() == 1) {
  Iterator iter = prop.getValues();
  while (iter.hasNext()) {
    val = (IPropertyValue) iter.next();
    break;
  }
}
// do something with the value
                
but I do not understand a reason why then we refuse to help them with
this:

if (prop.getPropertyModel().getMaxCardinality() == 1) {
  Iterator iter = prop.getValues();
  if (iter.hasNext()) {
    val = (IPropertyValue) iter.next();
    val.remove();
    break;
  }
  val = prop.addValue(...);
  // set value's data
}


Valery

Saturday, April 28, 2007, 8:14:42 PM, you wrote:

>  
>  
> Thanks Valery.
>  
>  
>  
> I understand the issue you present.  I think where we differ is in
> our views of the importance of the issue.
>  
>  
>  
> I think this kind of thing happens all the time in OO design when dealing with specialization.
>  
>  
>  
> For example, one could say that all Mammals can reproduce, and thus
> have an addChild method.  Let's specialize Mammals to Humans, and
> further, to ChineseCitizen.  There is local policy for the which
> says ChineseCitizen.addChild may only be called once.  The
> temptation remains, as a ChineseCitizen is a specialization of
> Mammal. Nevertheless, an error occurs when addChild is called multiple times.
>  
>  
>  
> I think people learn to expect this kind of thing, and deal with it all the time.
>  
>  
>  
> If we apply proposal 5 to the Polygon analogy, it would have us
> create a a Henagon class, a Triangle class, and Polygon class all at
> the same hierarchy.  To avoid temptation, Henagon would have
> setLineSegment(), Triangle would have setLineSegmentA(),
> setLineSegmentB(), and setLineSegmentC(), while polygon would have addLineSegment()
>  
>  
>  
> This (I believe) is not the way people typically model things.
>  
>  
>  
> So, at this point I have to say, while I acknowledge the temptation
> issue in having addValue on ISingleValuedProperty, I think it's
> consistent with OO design.  I'll let other people's feedback drive
> the direction here since it seems I may be biased.
>  
>  
>  
> Jim
>  


>>>> Valery Kokhan <vkokhan@xxxxxxxxxxxxxx> 4/28/07 10:22 AM >>>
> The difference between #3 and #5 is that both single and multi valued
> properties are on the same level of hierarchy and if I have single
> valued property I do not have a temptation to add value I can only set
> it.

> Valery

>> I like the motorcycle analogy, but I don't see it that way at all.
>>  
>>  
>>  
>> In proposal 3, what we've done is defined a WheeledVehicle, and
>> subclassed that with a Unicycle.   I'm not sure what functionality
>> exists in IProperty that is inappropriate for
>> ISingleValuedProperty.  The one which was mentioned was addValue. 
>> This is valid to be called once-only on a ISingleValuedProperty,
>> just as it is valid to be called 5 times-only on an IProperty which
>> has maxCardinality = 5.  Remember that ISingleValuedProperty is
>> nothing more than an IProperty with maxCardinality = 1.
>>  
>> In proposal 5 below, ISingleValuedProperty needs an addValue(URI
>> type) (which yes, could be called setValue I suppose), and
>> IMultiValuedProperty needs an addValue(IPropertyValue newValue) to
>> be consistent with other interfaces.  
>>  
>>  
>>  
>> I may be missing something, but I think proposal 5 is like proposal
>> 3 other than it doesn't use inheritance and uses a different name
>> for the setter (set instead of add).



>>
>> Jim
>>  

>>>>> Valery Kokhan <vkokhan@xxxxxxxxxxxxxx> 4/28/07 7:21 AM >>>
>> Actually I dislike proposal #3 because to my mind it is even more
>> confusing then #2. It looks like in #3 we're going to do something
>> like this:

>> We need to define an interface to represent a motorcycle with the
>> methods to fuel up, start the engine and drive and we define it as our
>> base interface (just because our model allow this). Then, because
>> there are use cases when we don't need an engine at all, we "extend"
>> our base interface by defining an interface to represent a cycle (just
>> for a user convenience). We are saying that cycle is actually a
>> motorcycle but... without an engine.

>> I hate to think that when I have an instance of the cycle I still have
>> a methods to fuel it up and to start an engine but with unknown
>> semantic however.

>> It looks like nobody like proposal #2 because of the way how
>> objects are extended there. Ok, I could live with that but as for me
>> #2 is less confusing then #3.

>> As alternative take a look at #5 below which to my mind could help us
>> to avoid ambiguity like we have in both #2 and #3.

>> Proposal 5:

>> Definitions:

>> interface IProperty {
>>         public URI getType() throws IdASException;
>>         public boolean isSingleValued();
>> }

>> interface ISingleValuedProperty extends IProperty {
>>         IPropertyValue getValue();
>>         IPropertyValue setValue(IPropertyValue newValue);
>> }

>> interface IMultiValuedProperty extends IProperty {
>>         Iterator getValues();
>>         IPropertyValue addValue(URI type);
>> }

>> Some sample code:

>> IProperty prop;
>> ...
>>                 
>> IPropertyValue val;
>> IPropertyValue newVal;
>>                 
>> if (prop.isSingleValued()) {
>>   ISingleValuedProperty sProp = (ISingleValuedProperty) prop;
>>   val = sProp.getValue();
>>   //do thimething with the value
>>                         
>>   //set new value
>>   sProp.setValue(newVal);
>> } else {
>>   IMultiValuedProperty mProp = (IMultiValuedProperty) prop;
>>   Iterator iter = mProp.getValues();
>>   while (iter.hasNext()) {
>>     val = (IPropertyValue)iter.next();
>>     // do something with the value
>>   }
>>                         
>>   //add new value
>>   mProp.addValue(URI.create("someType"));
>> }

>> In order, I'd prefer #5, #4, #2, #3, #1

>> Valery


>> Friday, April 27, 2007, 11:34:49 PM, you wrote:

>>>  
>>>  
>>> So, here's my opinion:
>>>  
>>> Proposal #1; I agree seems a bit wishy washy, but it's very simple
>>> (check for single-valuedness, and proceed with no casting)
>>>  
>>>  
>>>  
>>> Proposal #2; I like that there's no casting when it's a single-valued attribute.
>>>  
>>> I dislike because it presents a model where:
>>>  
>>> * A property has a value
>>>  
>>> * A value is either a value or a list of values
>>>  
>>> * A list of values is both a value and a list of values
>>>  
>>>  
>>>  
>>> It's the last point that I worry about.  A list of values is itself
>>> a value?  It means that my list has methods like isList(), and
>>> getType().  It also has isSimple() and isComplex().  I view the two
>>> latter as things that should be declared at the attribute level, not
>>> the value level.  Also, I don't want to present APIs that make it
>>> appear that we allow an attribute to contain different typed
>>> values.  It also presents the view that a value may be a list of
>>> values, each of which itself could be a list of values and so on. 
>>> Meaning, it presents an API view inconsistent with the data model.
>>>  
>>>  
>>>  
>>> Proposal #3; If we really think it's necessary to provide a
>>> convenience for users to call "getValue()" IFF an attribute is
>>> single-valued, then I slightly prefer this method over #1.  Valery
>>> dislikes that it's possible to call ISingleValuedProperty.setValue
>>> repeatedly (would fail on subsequent calls).  I think this is fine
>>> -- it's simply one of many places where an API will fail due to the
>>> constraints of a specific Context's model.
>>>  
>>>  
>>>  
>>> Proposal #4; This has the advantage of presenting exactly the
>>> intent of the data model.  Adding a convenience like getValue() is
>>> effectively trying to reflect the reality of a specific Context's
>>> model through the general-use IdAS APIs -- I believe this is why
>>> we're getting into trouble in the first place.  None of the other
>>> methods provide instructions that the consumer can't perform
>>> himself.  Further, the consumer can build his own convenience method however he wishes.
>>>  
>>>  
>>>  
>>> In order, I prefer #4, #3, #1, #2
>>>  
>>>  
>>>  
>>> Jim

>>>>>> "Jim Sermersheim" <jimse@xxxxxxxxxx> 4/27/07 12:14 PM >>>
>>>  
>>> 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)
>>>  
>>> http://download.eclipse.org/technology/higgins/idas/lastNightlyBuild/javadoc/org/eclipse/higgins/idas/IProperty.html#getValue(boolean)
>>>  
>>> 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
>>>  
>>> http://wiki.eclipse.org/index.php?title=IdAS_Update_Proposals_2 (Stuff in the first grey box)
>>>  
>>> 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
>>>  
>>> http://wiki.eclipse.org/index.php?title=IdAS_Update_Proposals_2 (Stuff in the second grey box)
>>>  
>>> 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.

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

>>   

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

>