Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re[4]: Fwd: Re: Re[2]: [higgins-dev] IdAS Update Proposals

Cool, can we agree on a proposal then?  I think the replace discussion was
tangential to that, right?  My tally yields proposal #3 with that one not
being completely opposed by anyone.

Tom

>>> Valery Kokhan <vkokhan@xxxxxxxxxxxxxx> 4/29/2007 3:04 AM >>>
This is exactly what I had in mind.

Valery

>  
>  
> Valery, after re-reading your code, I see something I didn't
> before.  You do want a convenient way to replace, but you also want
> it to work when no value yet exists.  We can say that
> ISingleValuedProperty::replaceValue may be called and treated as an
> add when the value is empty -- I've seen this in other APIs (like JNDI for example).
>  
>  
>  
> Jim

>>>> "Jim Sermersheim" <jimse@xxxxxxxxxx> 4/28/07 6:00 PM >>>
>  
> To restate, I think you're asking for a convenient way to replace a value. 
>  
>  
>  
> In both examples, the user performs the same steps to get the value which is everything up to:
>  
>     val = (IPropertyValue) iter.next();
>  
> If we give them a convenient way to get to that point, then replacing the value consists of:
>  
>   val.remove();
>   val = prop.addValue(...);
>   // set value's data
>  
> Now, if this is a simple value, they only have to call:
>  
>   val.setData(Object);
>  
> That is a replace (when the val already had data)
>  
>  
>  
> If it's a complex value, do they want to replace the entire value,
> or a component?  If they want to replace the entire value, they have
> to do the two steps above (val.remove(), then val = prop.addValue()).
>  
>  
>  
> Should we create a replace method so they can do this in one step? 
> If we do, should it be at the property, or at the value?  If at the
> property, the user has to somehow identify the value being
> replaced.  If we follow Proposal 3 and wanted to add this extra convenience, we would add these:
>  
>  
>  
> IPropertyValue IProperty::replaceValue(IPropertyValue oldVal, IPropertyValue newVal)
>  
>  
> IPropertyValue IProperty::replaceValue(IPropertyValue oldVal, URI newValType)
>  
> IPropertyValue ISingleValuedProperty::replaceValue(IPropertyValue newVal)
>  
>  
> IPropertyValue ISingleValuedProperty::replaceValue(URI newValType)
>  
>  
>  
> I suspect any of the proposals have ways of making a replace
> easier.  Maybe we should agree on one proposal, and then look at
> whether we need to add a replace convenience function for cases
> where we have a single, complex value.
>  
>  
>  
> Jim

>>>> Valery Kokhan <vkokhan@xxxxxxxxxxxxxx> 4/28/07 4:21 PM >>>
> 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 

>>   

> _______________________________________________
> 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


Back to the top