Smarter ItemProviderAdapter.hasChildren() implementation [message #415539] |
Thu, 20 December 2007 11:00 |
|
Hi,
I've realized that the current (2.4M4) implementation of the
ItemProviderAdapter.hasChildren() method is simple but not very
efficient. Especially in cases where the underlying object knows the
sizes of all containment features but needs do expensive network
requests to fill the collections with proper values. Currently I inject
an own ItemProviderAdapter base class into the hierarchy of the
generated item providers which avoids to access the children themselves:
public class CDOItemProviderAdapter extends ItemProviderAdapter
{
public CDOItemProviderAdapter(AdapterFactory adapterFactory)
{
super(adapterFactory);
}
@Override
@SuppressWarnings("deprecation")
public boolean hasChildren(Object object)
{
Collection<? extends EStructuralFeature> anyChildrenFeatures =
getChildrenFeatures(object);
if (anyChildrenFeatures.isEmpty())
{
anyChildrenFeatures = getChildrenReferences(object);
}
EObject eObject = (EObject)object;
for (EStructuralFeature feature : anyChildrenFeatures)
{
if (feature.isMany())
{
List<?> children = (List<?>)eObject.eGet(feature);
if (!children.isEmpty())
{
return true;
}
}
else
{
if (eObject.eIsSet(feature))
{
return true;
}
}
}
return false;
}
}
This method seems to work pretty well for me. Do you see any obstacles
with it?
Would it be possible to change the EMF code accordingly?
If so, be aware that the first five lines of my method only replicate
the function of the private getAnyChildrenFeatures() method!
Regards,
Eike Stepper
----
http://wiki.eclipse.org/CDO
http://wiki.eclipse.org/Net4j
Cheers
/Eike
----
http://www.esc-net.de
http://thegordian.blogspot.com
http://twitter.com/eikestepper
|
|
|
|
Re: Smarter ItemProviderAdapter.hasChildren() implementation [message #415555 is a reply to message #415545] |
Thu, 20 December 2007 15:00 |
|
Ed Merks schrieb:
> Eike,
>
> Comments below.
>
>
> Eike Stepper wrote:
>> Hi,
>>
>> I've realized that the current (2.4M4) implementation of the
>> ItemProviderAdapter.hasChildren() method is simple but not very
>> efficient.
> I can imagine that for lazy loading situations asking for the actual
> children is less than optimal. The design does ensure that no matter
> how folks specialize getChildren, the hasChildren will be correct and
> won't require parallel changes.
I understand this although I'd be happier to have efficient versions and
force (JavaDoc) an overwriter of getChildren() to at least think about
an efficient version of hasChildren() for his getChildren(). But that's
quite subjective.
>> Especially in cases where the underlying object knows the sizes of
>> all containment features but needs do expensive network requests to
>> fill the collections with proper values. Currently I inject an own
>> ItemProviderAdapter base class into the hierarchy of the generated
>> item providers which avoids to access the children themselves:
>>
>> public class CDOItemProviderAdapter extends ItemProviderAdapter
>> {
>> public CDOItemProviderAdapter(AdapterFactory adapterFactory)
>> {
>> super(adapterFactory);
>> }
>>
>> @Override
>> @SuppressWarnings("deprecation")
>> public boolean hasChildren(Object object)
>> {
>> Collection<? extends EStructuralFeature> anyChildrenFeatures =
>> getChildrenFeatures(object);
>> if (anyChildrenFeatures.isEmpty())
>> {
>> anyChildrenFeatures = getChildrenReferences(object);
>> }
>>
>> EObject eObject = (EObject)object;
>> for (EStructuralFeature feature : anyChildrenFeatures)
>> {
>> if (feature.isMany())
>> {
>> List<?> children = (List<?>)eObject.eGet(feature);
>> if (!children.isEmpty())
>> {
>> return true;
>> }
>> }
>> else
>> {
>> if (eObject.eIsSet(feature))
>> {
>> return true;
>> }
>> }
>> }
>>
>> return false;
>> }
>> }
>>
>> This method seems to work pretty well for me. Do you see any
>> obstacles with it?
> That seems like a good approach. I could imagine though that the
> default value of a feature might be displayed as a child...
I don't understand the part about the default value. Do you say that the
current implementation doesn't show certain children because they
represent a default value? If it's "only" a default how could it be
changed or removed if it's not visible at the same time?
>> Would it be possible to change the EMF code accordingly?
> Possible, but you could imagine that breaking clients. Perhaps we
> could provide the implementation of the method but leave it to clients
> to configure something to actually use it. You'd likely still want
> your own root class for that...
I think having the impl in ItemProviderAdapter would be good, especially
due to the possible reuse of getAnyChildrenFeatures(). Ahh, and I don't
have to maintain it ;-)
I filed https://bugs.eclipse.org/bugs/show_bug.cgi?id=213592
Regards,
Eike Stepper
----
http://wiki.eclipse.org/CDO
http://wiki.eclipse.org/Net4j
Cheers
/Eike
----
http://www.esc-net.de
http://thegordian.blogspot.com
http://twitter.com/eikestepper
|
|
|
Re: Smarter ItemProviderAdapter.hasChildren() implementation [message #415557 is a reply to message #415555] |
Thu, 20 December 2007 15:07 |
Ed Merks Messages: 33140 Registered: July 2009 |
Senior Member |
|
|
This is a multi-part message in MIME format.
--------------010308070500030307070401
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Eike,
Comments below.
Eike Stepper wrote:
> Ed Merks schrieb:
>> Eike,
>>
>> Comments below.
>>
>>
>> Eike Stepper wrote:
>>> Hi,
>>>
>>> I've realized that the current (2.4M4) implementation of the
>>> ItemProviderAdapter.hasChildren() method is simple but not very
>>> efficient.
>> I can imagine that for lazy loading situations asking for the actual
>> children is less than optimal. The design does ensure that no matter
>> how folks specialize getChildren, the hasChildren will be correct and
>> won't require parallel changes.
> I understand this although I'd be happier to have efficient versions
> and force (JavaDoc) an overwriter of getChildren() to at least think
> about an efficient version of hasChildren() for his getChildren(). But
> that's quite subjective.
With hindsight that's certainly a reasonable approach too, but generally
I tried to avoid breaking clients with subsequent changes, which
sometimes prevents us from making good changes...
>
>>> Especially in cases where the underlying object knows the sizes of
>>> all containment features but needs do expensive network requests to
>>> fill the collections with proper values. Currently I inject an own
>>> ItemProviderAdapter base class into the hierarchy of the generated
>>> item providers which avoids to access the children themselves:
>>>
>>> public class CDOItemProviderAdapter extends ItemProviderAdapter
>>> {
>>> public CDOItemProviderAdapter(AdapterFactory adapterFactory)
>>> {
>>> super(adapterFactory);
>>> }
>>>
>>> @Override
>>> @SuppressWarnings("deprecation")
>>> public boolean hasChildren(Object object)
>>> {
>>> Collection<? extends EStructuralFeature> anyChildrenFeatures =
>>> getChildrenFeatures(object);
>>> if (anyChildrenFeatures.isEmpty())
>>> {
>>> anyChildrenFeatures = getChildrenReferences(object);
>>> }
>>>
>>> EObject eObject = (EObject)object;
>>> for (EStructuralFeature feature : anyChildrenFeatures)
>>> {
>>> if (feature.isMany())
>>> {
>>> List<?> children = (List<?>)eObject.eGet(feature);
>>> if (!children.isEmpty())
>>> {
>>> return true;
>>> }
>>> }
>>> else
>>> {
>>> if (eObject.eIsSet(feature))
>>> {
>>> return true;
>>> }
>>> }
>>> }
>>>
>>> return false;
>>> }
>>> }
>>>
>>> This method seems to work pretty well for me. Do you see any
>>> obstacles with it?
>> That seems like a good approach. I could imagine though that the
>> default value of a feature might be displayed as a child...
> I don't understand the part about the default value. Do you say that
> the current implementation doesn't show certain children because they
> represent a default value? If it's "only" a default how could it be
> changed or removed if it's not visible at the same time?
Note *this *part of the implementation for getting the children, which
doesn't use eIsSet as a guard:
public Collection<?> getChildren(Object object)
{
ChildrenStore store = getChildrenStore(object);
if (store != null)
{
return store.getChildren();
}
store = createChildrenStore(object);
List<Object> result = store != null ? null : new
ArrayList<Object>();
EObject eObject = (EObject)object;
for (EStructuralFeature feature : getAnyChildrenFeatures(object))
{
if (feature.isMany())
{
List<?> children = (List<?>)eObject.eGet(feature);
int index = 0;
for (Object unwrappedChild : children)
{
Object child = wrap(eObject, feature, unwrappedChild, index);
if (store != null)
{
store.getList(feature).add(child);
}
else
{
result.add(child);
}
index++;
}
}
else
{
* Object child = eObject.eGet(feature);
if (child != null)*
{
child = wrap(eObject, feature, child,
CommandParameter.NO_INDEX);
if (store != null)
{
store.setValue(feature, child);
}
else
{
result.add(child);
}
}
}
}
return store != null ? store.getChildren() : result;
}
>
>>> Would it be possible to change the EMF code accordingly?
>> Possible, but you could imagine that breaking clients. Perhaps we
>> could provide the implementation of the method but leave it to
>> clients to configure something to actually use it. You'd likely
>> still want your own root class for that...
> I think having the impl in ItemProviderAdapter would be good,
> especially due to the possible reuse of getAnyChildrenFeatures(). Ahh,
> and I don't have to maintain it ;-)
Yep, I think that makes it easier for clients.
>
> I filed https://bugs.eclipse.org/bugs/show_bug.cgi?id=213592
>
> Regards,
> Eike Stepper
> ----
> http://wiki.eclipse.org/CDO
> http://wiki.eclipse.org/Net4j
>
--------------010308070500030307070401
Content-Type: text/html; charset=ISO-8859-15
Content-Transfer-Encoding: 8bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-15"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Eike,<br>
<br>
Comments below.<br>
<br>
Eike Stepper wrote:
<blockquote cite="mid:fke02j$37n$2@build.eclipse.org" type="cite">Ed
Merks schrieb:
<br>
<blockquote type="cite">Eike,
<br>
<br>
Comments below.
<br>
<br>
<br>
Eike Stepper wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
I've realized that the current (2.4M4) implementation of the
ItemProviderAdapter.hasChildren() method is simple but not very
efficient.
<br>
</blockquote>
I can imagine that for lazy loading situations asking for the actual
children is less than optimal.
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
Powered by
FUDForum. Page generated in 0.03065 seconds