Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: Add a convenience method for the full set of types for a type #50

Open
ljnelson opened this issue Feb 22, 2020 · 9 comments
Open
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest Issue related to Hactoberfest2020 activities, eligible for additional rewards

Comments

@ljnelson
Copy link

ljnelson commented Feb 22, 2020

Thanks for a great project!

It would be helpful if there were a method on ResolvedType that could provide a List or Set of all implemented and extended Types.

I know that ResolvedType has getImplementedInterfaces(). And I know that TypeResolver has getParentClass(). It would be nice if there were a method like public Set<ResolvedType> getAllTypes() or something of that nature implemented in terms of the other two.

I would expect, for example, that if it were passed String.class the returned set would include types representing:

  • Object
  • Comparable<String>
  • CharSequence
  • String
  • Serializable
@cowtowncoder
Copy link
Member

No problem! Glad you find it useful -- it seemed like something that was missing, and was sort of fun to extract out of Jackson (or rather... try to extract, realize how much it needed full rewrite... and eventually while Jackson does not use it as is due to compatibility reasons, also provided a few fixes to Jackson itself).

Now, convenience method(s) sound(s) reasonable. Ideally I guess it/those would be via ResolvedType, if possible (I think types are fully resolved meaning that it should be possible).

I am not sure how much time I have to work on this, but typically I do try to follow-up on PRs.
But I will add this on something to evaluate wrt my own external TODO list, so that at least there's a chance I might pick it up as a sort of lower-hanging fruit.

@cowtowncoder cowtowncoder added good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest Issue related to Hactoberfest2020 activities, eligible for additional rewards labels Oct 13, 2020
krzysiekbielicki added a commit to krzysiekbielicki/java-classmate that referenced this issue Oct 15, 2020
krzysiekbielicki added a commit to krzysiekbielicki/java-classmate that referenced this issue Oct 15, 2020
krzysiekbielicki added a commit to krzysiekbielicki/java-classmate that referenced this issue Oct 15, 2020
@cowtowncoder
Copy link
Member

Thinking about this a bit, I think it'd be useful to have couple of different new methods:

  1. getAllParentClasses() (does not include interfaces)
  2. getAllImplementedInterfaces() (no parent classes, but recursive)
  3. getAllSuperTypes() -- both of above, the original ask more or all
  4. getAllErasedSuperTypes() like (3) but with Class<?>

and possibly helper variants that take Set to add parents to: this allows caller to relatively easily pre-populate list with type itself if that is desired (sometimes is, sometimes now).

For now I'll consider this to be just for (3), but I may want to make bigger addition.

As to return type, I think Set makes most sense just since implementation pretty much must check whether type is already included (to avoid recursion, possibly, and to avoid double inclusion).

@ljnelson
Copy link
Author

Do consider defining (in javadoc) an order to the Set even if you don't specify what it is. There was a fun little Weld issue at one point that turned out to be dependent on a random order in a Set of Types related to CDI beans.

@cowtowncoder
Copy link
Member

I could specify use of LinkedHashSet I suppose; although if there is parallel method that takes in Set to add entries to, caller could enforce particular ordering.

But it almost seems like if caller really wants specific ordering, may be their should implement traversal themselves?

@ljnelson
Copy link
Author

…although to be fair if they're implementing traversal themselves…then why…are they using this framework 😄 😄 😄

@cowtowncoder
Copy link
Member

I meant traversal of resolved super types, to get the collection in exact order caller wants -- versus just getting a Set/Collection of super types in arbitrary order.
I can think of many use cases of latter, and some for former: but in latter case useful ordering possibilities are unbounded.

I am just not sure how much can or should be guaranteed about order of entries when I am not really sure what ordering(s) would be useful -- when combining interfaces and super classes, for example, which should come first?

@ljnelson
Copy link
Author

Sure; was just having a little bit of fun. I think what's important is that there is an order, not so much what the order is. The Weld/CDI case I was thinking of had to do with proxy class naming: they build up a proxy class name out of the types and interfaces in the type closure. Obviously if that order changes then you end up creating proxy classes instead of loading existing ones etc.

@cowtowncoder
Copy link
Member

Heh. Yeah, just wanted to make sure. I can see benefit of stable ordering, but now that I think of this, I am not sure how stable it can be.

Specifically: traversal order can't really be guaranteed for, say, implemented interface as JDK does not expose interfaces in guaranteed order AFAIK. One could of course sort them explicitly by classname.
Alternatively sorting just ResolvedType would be fine, except it does not implement Comparable.

Although... maybe it should? Sort by classname and be done? Then simple TreeSet would work (... although not super efficient for this purpose)

Another issue to file, perhaps :)

@ljnelson
Copy link
Author

Yes, exactly. That's how I ended up doing it, with the same reservations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project hacktoberfest Issue related to Hactoberfest2020 activities, eligible for additional rewards
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants