add ClassInfo.canonicalConstructor() #419
Conversation
| */ | ||
| public MethodInfo canonicalConstructor() { | ||
| if (!isRecord()) { | ||
| return null; |
There was a problem hiding this comment.
Isn't it better to throw an exception here?
There was a problem hiding this comment.
In Jandex, it is customary to not throw but return a sentinel value in such cases. For example, ClassInfo.recordComponents() doesn't throw on non-records, it just returns an empty list.
We do throw an exception at the end of this method, but that's only for truly exceptional situations.
There was a problem hiding this comment.
Ok, java.lang.Class.getRecordComponents() also returns null. Sometimes we don't retun null, e.g. empty list in case of ClassInfo#enumConstants() but that's fine. TBH there should be probably RecordInfo and EnumInfo instead but 🤷.
There was a problem hiding this comment.
Yeah, that would also require AnnotationInfo and InterfaceInfo for completeness :-)
There was a problem hiding this comment.
Is there any way to make it clear from the name that this only works for records?
I'm not sure canonical will be clear enough for us laymen.
There was a problem hiding this comment.
That is a good point. It should be canonicalRecordConstructor()!
There was a problem hiding this comment.
It seems to me that canonicalConstructor() is enough, but canonicalRecordConstructor() is good too. Will rename.
There was a problem hiding this comment.
Yeah, agreed. So far, only records have canonical constructors. People might wonder what a non-record canonical constructor is, but it doesn't exist. So +1 on the changed name.
No description provided.