> That was not easy to do with the current grammar hence the limitation. I
was hoping we would find a way to remove the limitation before 4GA hence it
possibly isn't mentioned yet in the documentation/release notes. The
working of how the compact constructor was "plumbed in" has changed
slightly - perhaps we can re-look whether removing that limitation is
easier now.
Too busy recently... I will try to find some spare time to look into the issue.
Cheers,
Daniel Sun
On 2021/11/10 08:22:35 Paul King wrote:
> Comments inline.
>
> On Wed, Nov 10, 2021 at 1:17 AM Milles, Eric (TR Technology) <
> eric.mil...@thomsonreuters.com> wrote:
>
> > A couple questions regarding record types:
> >
> >
> >
> > record Person(String name, Date dob) {
> >
> > public Person {
> >
> > // ...
> >
> > }
> >
> > }
> >
> >
> >
> > 1) Was it by design to have Person(String) and Person() constructors
> > created for the example above? I would prefer to see the canonical
> > constructor and the map constructor only. That is,
> > TupleConstructor(defaults=false). Then if I wanted the others, I can add
> > default arguments to any of the components or set defaults to true
> > explicitly.
> >
>
> The current implementation is by design. "@RecordType" expands to (among
> other things) "@TupleConstructor(namedVariant = true, force = true)" and
> doesn't include "defaults = false". The way we do our merging given we
> currently have PREFER_EXPLICIT_MERGED as the collector mode meant that if I
> had "defaults = false" explicitly it wasn't allowing "defaults = true" to
> be added. Merging was overriding it. We possibly need to re-look at that
> merging behavior and then flipping the defaults boolean is a workable way
> forward.
>
>
> > 2) Is the modifier required for the compact constructor by design? If I
> > remove "public" above it fails to parse. Java allows the compact
> > constructor without visibility modifier.
> >
>
> That was not easy to do with the current grammar hence the limitation. I
> was hoping we would find a way to remove the limitation before 4GA hence it
> possibly isn't mentioned yet in the documentation/release notes. The
> working of how the compact constructor was "plumbed in" has changed
> slightly - perhaps we can re-look whether removing that limitation is
> easier now.
>
>
> > 3) Did the copyWith() or components() ideas get dropped? I am not seeing
> > them in 4.0b2.
> >
>
> They currently default to false but you can turn them on with the
> respective boolean annotation attributes in RecordOptions. I set them to
> false because:
> * copyWith() is also false in ImmutableBase, so I left it at the same
> default value as that for consistency and you can get a long way with
> toList/toMap and the existing constructors.
> * components() only works for records having a limited number of components
> up to our largest TupleN class (currently Tuple16), so I figured users who
> turn that on will know what they are doing and won't be surprised if they
> get a compiler error for 17 components (say)
>
> 4) Since the compact constructor is implemented via
> > TupleConstructor(pre=...), there is no error or warning if I provide a pre
> > closure and a compact constructor. Should there be an error or warning?
> >
>
> I am not sure it would always be an error/warning or if some natural
> merging could be feasible but I suspect it isn't covered well by existing
> tests. Any help in that area would be greatly appreciated.
>
>
>
> >
> >
> >
> >
> > *From:* OCsite <o...@ocs.cz>
> > *Sent:* Tuesday, November 2, 2021 2:07 PM
> > *To:* MG <mg...@arscreat.com>
> > *Cc:* Groovy_Developers <dev@groovy.apache.org>; pa...@asert.com.au
> > *Subject:* [EXT] Re: Record enhancements
> >
> >
> >
> > *External Email:* Use caution with links and attachments.
> >
> >
> >
> > As for tersity, I presume the actual usage would look like „foo as Map“ or
> > „foo as List“ anyway, which is actually one less keypress :), and — which
> > in my personal opinion is considerably more important — it offers better
> > consistency and polymorphism.
> >
> >
> >
> > (I know next to nothing of Intellisense, but I guess it should offer the
> > *as* operator with a selection of known types, should it not?)
> >
> >
> >
> > Still I might be missing something of importance, of course.
> >
> >
> >
> > All the best,
> >
> > OC
> >
> >
> >
> > On 2 Nov 2021, at 19:17, MG <mg...@arscreat.com> wrote:
> >
> >
> >
> > Hmmm, yes, that would be an option.
> > More terse & can be discovered via Intellisense are two reasons I could
> > think of that speak for the toList()/toMap() approach...
> >
> > Cheers,
> > mg
> >
> > On 02/11/2021 12:48, OCsite wrote:
> >
> > Hi there,
> >
> > I am probably missing something obvious here, but why adding separate
> > methods for this instead of simply reusing asType?
> >
> > Thanks and all the best,
> > OC
> >
> >
> > On 2. 11. 2021, at 8:35, Paul King <pa...@asert.com.au> wrote:
> >
> > Thanks for the feedback! I added "toMap()" to the PR.
> >
> > On Tue, Nov 2, 2021 at 7:02 AM MG <mg...@arscreat.com> wrote:
> >
> > Hi Paul,
> >
> > quick "from the top of my head" reply:
> >
> > copyWith(...): Sounds like a great idea, I have record-like classes in
> > use, and the need for something like this arises immediately in practice
> > getAt(int): Don't see why not, might be useful
> > toList(): Destructuring should show its strength when pattern matching is
> > introduced - outside of pattern matching right now I don't see much
> > application/need for such functionality (but would be interested to see
> > some practical examples :-) ), but having it does not seem to hurt.
> > components(): Same as getAt; the name seems quite long, maybe we can come
> > up with something more terse ?
> > toMap(): If we have toList(), would a toMap() make sense, so that the map
> > could be modified and passed as a record ctor argument to create a new
> > record ?
> >
> > Cheers,
> > mg
> >
> >
> > On 01/11/2021 16:14, Paul King wrote:
> >
> > Hi folks,
> >
> > I will be ready for a new Groovy 4 release shortly. I am interested in
> > folks' thoughts on records as they have gone through a few changes
> > recently (documented in [1], [2] and [3]) and there is a proposal[4]
> > for a few more enhancements.
> >
> > There is a "copyWith" method (still undergoing some refactoring)
> > similar to the copy method in Scala and Kotlin which allows one record
> > to be defined in terms of another. It can be disabled if you really
> > must have Java-like records. The refactoring of that method hit a
> > slight glitch, so might not work if you grab the latest source but
> > should be fixed shortly.
> >
> > record Fruit(String name, double price) {}
> > def apple = new Fruit('Apple', 11.6)
> > assert apply.toString() == 'Fruit[name=Apple, price=11.6]'
> > def orange = apple.copyWith(name: 'Orange')
> > assert orange.toString() == 'Fruit[name=Orange, price=11.6]'
> >
> > There is a "getAt(int)" method to return e.g. the first component with
> > myRecord[0] following similar Groovy conventions for other aggregates.
> > This is mostly targeted at dynamic Groovy as it conveys no typing
> > information. Similarly, there is a "toList" (current name but
> > suggestions welcome) method which returns a Tuple (which is also a
> > list) to return all of the components (again with typing information).
> >
> > record Point(int x, int y, String color) {}
> > def p = new Point(100, 200, 'green')
> > assert p[0] == 100
> > assert p[1] == 200
> > assert p[2] == 'green'
> > def (x, y, c) = p.toList()
> > assert x == 100
> > assert y == 200
> > assert c == 'green'
> >
> > There is also an optional (turned on by an annotation attribute)
> > "components" method which returns all components as a typed tuple,
> > e.g. Tuple1, Tuple2, etc. This is useful for Groovy's static nature
> > and is automatically handled by current destructuring (see the tests
> > in the PR). The limitation is that we currently only go to Tuple16
> > with our tuple types - which is why I made it disabled by default.
> >
> > @RecordBase(componentTuple=true)
> > record Point(int x, int y, String color) { }
> >
> > @TypeChecked
> > def method() {
> > def p1 = new Point(100, 200, 'green')
> > def (int x1, int y1, String c1) = p1.components()
> > assert x1 == 100
> > assert y1 == 200
> > assert c1 == 'green'
> >
> > def p2 = new Point(10, 20, 'blue')
> > def (x2, y2, c2) = p2.components()
> > assert x2 * 10 == 100
> > assert y2 ** 2 == 400
> > assert c2.toUpperCase() == 'BLUE'
> > }
> >
> > An alternative would be to follow Kotlin's approach and just have
> > typed methods like "component1", "component2", etc. We might want to
> > follow that convention or we might want to follow our TupleN naming,
> > e.g. "getV1", "getV2", etc. We would need to augment the Groovy
> > runtime and type checker to know about records if we wanted to support
> > destructuring but we could avoid the "toList" method and "components"
> > method with its size limitation if we did add such support.
> >
> > Any feedback welcome,
> >
> > Cheers, Paul.
> > P.S. Records are an incubating feature - hence may change in backwards
> > incompatible ways, particularly until we hit Groovy 4 final.
> >
> > [1]
> > https://github.com/apache/groovy/blob/master/src/spec/doc/_records.adoc
> > <https://urldefense.com/v3/__https:/github.com/apache/groovy/blob/master/src/spec/doc/_records.adoc__;!!GFN0sa3rsbfR8OLyAw!NyOeTO41BsAx72RYKaFkHpjzc7rfbQO2ajvOqriwA1vxqYxsGsMsTEVmIQKC-qtDO--XcfI$>
> > [2]
> > https://github.com/apache/groovy/blob/master/src/spec/test/RecordSpecificationTest.groovy
> > <https://urldefense.com/v3/__https:/github.com/apache/groovy/blob/master/src/spec/test/RecordSpecificationTest.groovy__;!!GFN0sa3rsbfR8OLyAw!NyOeTO41BsAx72RYKaFkHpjzc7rfbQO2ajvOqriwA1vxqYxsGsMsTEVmIQKC-qtDCIm0GQs$>
> > [3]
> > https://github.com/apache/groovy-website/blob/asf-site/site/src/site/wiki/GEP-14.adoc
> > <https://urldefense.com/v3/__https:/github.com/apache/groovy-website/blob/asf-site/site/src/site/wiki/GEP-14.adoc__;!!GFN0sa3rsbfR8OLyAw!NyOeTO41BsAx72RYKaFkHpjzc7rfbQO2ajvOqriwA1vxqYxsGsMsTEVmIQKC-qtDPjyENMk$>
> > [4] https://issues.apache.org/jira/browse/GROOVY-10338
> > <https://urldefense.com/v3/__https:/issues.apache.org/jira/browse/GROOVY-10338__;!!GFN0sa3rsbfR8OLyAw!NyOeTO41BsAx72RYKaFkHpjzc7rfbQO2ajvOqriwA1vxqYxsGsMsTEVmIQKC-qtDt01uPzI$>
> >
> >
> >
> >
> >
>
