[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re[5]: Fwd: Re: Re[2]: [higgins-dev] IdAS Update Proposals
|
Actually, I'd prefer #5 but I can live with this either if everybody
against #5.
Valery
Monday, April 30, 2007, 6:39:19 PM, you wrote:
> 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
