This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 74348 - Minor generification problems in AbstractLookup.Pair
Summary: Minor generification problems in AbstractLookup.Pair
Status: RESOLVED WONTFIX
Alias: None
Product: platform
Classification: Unclassified
Component: Lookup (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API
Depends on:
Blocks: 67888
  Show dependency tree
 
Reported: 2006-04-04 02:23 UTC by Jesse Glick
Modified: 2008-12-22 20:37 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Fix for the Lookups.lookupItem, I do not think the first problem is valid however (2.46 KB, patch)
2006-04-04 07:27 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2006-04-04 02:23:11 UTC
protected abstract boolean instanceOf(Class<?> c);
protected abstract boolean creatorOf(Object obj);

Should these be

protected abstract boolean instanceOf(Class<? extends T> c);
protected abstract boolean creatorOf(T obj);

?

Also Lookups.lookupItem looks wrong:

public static Lookup.Item lookupItem(Object instance, String id);

should be

public static <T> Lookup.Item<T> lookupItem(T instance, String id);

right?
Comment 1 Jaroslav Tulach 2006-04-04 07:27:31 UTC
Created attachment 29579 [details]
Fix for the Lookups.lookupItem, I do not think the first problem is valid however
Comment 2 Jaroslav Tulach 2006-04-04 08:17:52 UTC
I fixed the Lookups.lookupItem part:

openide/util/src/org/openide/util/lookup/Lookups.java,v  <--  Lookups.java
new revision: 1.4; previous revision: 1.3

As concern the Pair signature, I do not think so. Basically I can call the 
methods with any object or class without knowing the T. The case of creatorOf 
might be similar to Collection.remove(Object). However I am not 100% sure.
Comment 3 Jesse Glick 2006-04-04 14:11:37 UTC
I disagree: Pair<T> is subclassing Item<T> and does not even use its type
parameter at all! Also look at SimpleLookupTest to see why the current situation
is painful:

protected boolean creatorOf(Object obj) {
    // item.getInstance :: T, so why Object?
    return item.getInstance() == obj;
}

and

public Class<? extends T> getType() {
    return item.getType ();
}
@SuppressWarnings("unchecked") // !
protected boolean instanceOf(Class c) {
    return c.isAssignableFrom(getType());
}
Comment 4 Jesse Glick 2006-04-04 14:19:26 UTC
The lookupItem patch seems to work well.
Comment 5 Jesse Glick 2006-04-04 16:31:35 UTC
It is possible that instanceOf(Class<?>) is correct but creatorOf(T) is
preferable. TBD - depends on usage patterns, specifically whether Lookup impls
call these methods on the "wrong" pairs.

Related to that, wondering why an instanceOf impl would ever return something
other than c.isAssignableFrom(getType()). Is there some additional meaning to
this method? Are there circumstances where getType() is expensive but
instanceOf(Class) is cheap (FolderLookup perhaps)? Is getType()
required/expected to return the concrete impl class (i.e. maybe a subclass of T)
or can it just return T.class?

Adjusting summary since Lookups.lookupItem is OK now, just talking about
Pair<T>. And lowering priority since there are not many Pair subclasses outside
of openide/util to my knowledge; most people would use Lookups.* factories or
InstanceContent.
Comment 6 Jaroslav Tulach 2006-04-10 07:12:37 UTC
I guess I send an answer to your last question via email and I somehow feel 
there is not much to do about our current state.

FolderLookup.Item does handle instanceOf differently than getType().

I admit I am not 100% sure about creatorOf, especially because there is just 
one usage of it in AbstractLookup, but the javadoc correctly says what I meant 
- if the pair created an instances and it is == then the pair is creator. 

I believe that for usage of == the current types are more than appropriate.