jakartaee / expression-language

Jakarta Expression Language
https://eclipse.org/ee4j/el
Other
70 stars 50 forks source link

Make MethodReference more Record like #169

Closed Thihup closed 3 years ago

Thihup commented 3 years ago

As this is a new class, I guess it would be good if we could model it like a java.lang.Record:

This way, we can change it to record in the future, when it probably will be usable in Pattern Matching (like destructuring) and not have two accessors (base() and getBase()).

markt-asf commented 3 years ago

Hmm. I'm not sure. On one hand I like being ready for Record but on the other I dislike that this would make the API use inconsistent styles - especially because of the close relationship between MethodReference and MethodInfo.

It also assumes we'll want to make greater use of Record. Is that the case?

tandraschko commented 3 years ago

IMO a API style should be consistent, so therefore currently -1 to make it a record

Thihup commented 3 years ago

A compromise would be to declare the records in the following way:

record MethodReference(
    Object getBase,
    MethodInfo getMethodInfo,
    Annotation[] getAnnotations,
    Object[] getEvaluatedParameters){}

This way the API would be consistent. This would be possible to apply to the other classes. However, most of them are not final (like ELClass), so changing to use records would be a breaking change. Marking the MethodReference as final now would enable us to change it in the future without having breaking changes

tandraschko commented 3 years ago

TBH i really think that final classes (or records) should be avoided in almost all cases in a API like JakartaEE

there may be cases when someone would like to wrap or extend it, like MF did for core PropertyDescriptor (https://github.com/apache/myfaces/blob/master/api/src/main/java/org/apache/myfaces/core/api/shared/lang/LambdaPropertyDescriptor.java)

markt-asf commented 3 years ago

I think the extensibility argument is a good one so I am going to close this issue as "won't fix".