jandex icon indicating copy to clipboard operation
jandex copied to clipboard

Get rid of Set<ClassInfo> usages

Open mkouba opened this issue 3 years ago • 3 comments

I've found 22 usages of Set<ClassInfo> directly in jandex classes org.jboss.jandex.Index and org.jboss.jandex.CompositeIndex. Given the fact that ClassInfo does not implement equals/hashCode it would make sense to refactor these classes, esp. the CompositeIndex where multiple ClassInfo instances representing the same java class may appear.

mkouba avatar Feb 15 '22 14:02 mkouba

Those may actually be correct, because in absence of equals/hashCode, a HashMap essentially degenerates into an IdentityHashMap, which may be desired, especially in case of CompositeIndex. That said, I didn't look too deep, reviewing all the places is certainly a good idea.

Ladicek avatar Feb 28 '22 14:02 Ladicek

may be desired, especially in case of CompositeIndex

Could you be more specific?

TBH I find the fact that a CompositeIndex may contain multiple ClassInfos (same or different) of the same FQCN and the CompositeIndex.getClassByName(DotName) method just returns the first one found very confusing...

mkouba avatar Mar 08 '22 16:03 mkouba

I have the same feelings about CompositeIndex containing multiple equivalent but different classes, but I can't tell whether that was an intentional decision or not, and at this point, can't even rule out that there are people depending on it :-)

I mean, I agree it should be investigated, but I'm not 100% convinced yet that they all may safely be eliminated.

Ladicek avatar Mar 08 '22 16:03 Ladicek