Rename Composite and Variant type defs

[ascjones]

@Robbepop and I went back and forth over this originally and settled on Composite and Variant as non-rust specific names for struct and enum types.

However I think this naming is slightly awkward in places, especially when writing code for a "variant" type which itself contains "variants".

So I think we should just use the enum and struct terminology, people understand what they are and the code will be easier to read/write. Also that is the terminology used in the SCALE defitions: https://substrate.dev/docs/en/knowledgebase/advanced/codec#enumerations-tagged-unions

[dvdplm]

If we ever were to support unions, struct would be an awkward name (but I don't think we are going to support unions any time soon).

[jacogr]

100% on-board with this :) As an implementer it is actually quite awkward when starting to ue these since the terms are completely non-standard to what we use elsewhere. Would also get it in "now-ish" since, well, we have quite a large dependency on this now in metadata and it is the right time to break things.

[ascjones]

@Robbepop is strongly in favour of the current names, so we would need to persuade him.

[jacogr]

It is fine to keep if that is the way the scale-info API is meant to be.

The undeniable point is that it is absolutely terrible from an external API usability perspective and actually knowing what these are meant to represent. This is now the second time I'm coding against this API in anger and even with experience, it is still opaque.

After seeing this, I've actually started a PR to rename them in the JS API (with parsing aliases for e.g. JSON inputs), since these are now going to start being front-and-center and as exposed just doesn't reflect what it is meant to. Rather than explaining over-and-over in support issues (aka what does this mean in the metadata) renaming the mappings is just easier.

So at least on the JS API types side these will be more usable and maintainable without having a scale PhD, which solves the issue.

[Robbepop]

So at least on the JS API types side these will be more usable and maintainable without having a scale PhD, which solves the issue.

Composite and Variant are already compromises coming from Product and Sum type names which are more of the "PhD" terms for the same. After all we wanted to keep Rust out of SCALE codec and therefore we should use non-Rust (or non PL) specific terms since they would always confuse someone coming from yet another language.
What are the names your JS API proposes for those now?

[jacogr]

Since everybody looks at the Rust types anyway to grok in/out in Substrate, attempting to match the definition closer of the underlying types they refer to.

• Struct - as suggested above (it is not 100%, since unnamed can it refer to Tuple-like sequences of fields)
• Enum - as suggested above (mostly ok, although it also matches in actual use, enums that are used in sets, e.g. IdentityFields)
• Vec - additionally replacing sequence
• Array - this is to be replaced, no idea as of "what" (it has specific meaning more akin to Vec as currently named, however it is a fixed length sequence of items, here even the base SCALE implementation on the JS API is horrible named, e.g VecFixed which is less than optimal, but had no better ideas at the time and it sort-of stuck around)

YMMV. (The above will gets some tweaks over the next day or two, real focus is making sure it works end-to-end and finding any remaining nigglies, renaming is easy pre-merge into Substrate)

[Robbepop]

Since everybody looks at the Rust types anyway to grok in/out in Substrate, attempting to match the definition closer of the underlying types they refer to.

* Struct - as suggested above (it is not 100%, since unnamed can it refer to Tuple-like sequences of fields)

* Enum - as suggested above (mostly ok, although it also matches in actual use, enums that are used in sets, e.g. IdentityFields)

* Vec - additionally replacing sequence

* Array - this is to be replaced, no idea as of "what" (it has specific meaning more akin to Vec as currently named, however it is a fixed length sequence of items, here even the base SCALE implementation on the JS API is horrible named, e.g `VecFixed` which is less than optimal, but had no better ideas at the time and it sort-of stuck around)

YMMV. (The above will gets some tweaks over the next day or two, real focus is making sure it works end-to-end and finding any remaining nigglies, renaming is easy pre-merge into Substrate)

Let me quickly go over why I think those names are not a good idea.

• Struct as you said yourself this kind of confuses people by making them think about Rust structs without going beyond that tuple structs and tuples are included as well. You made this point yourself though.
• Enum: Many people coming from C/Java/C#/C++ won't recon enum as something that may hold data. Therefore for all of those people this is again a bad naming and confusing. Even some Rust people think that enum was a bad name for the construct.
• Vec instead of Sequence: This would give people the wrong impression that all the different collections that are serialized similar to a vector as in fact just vectors. So sequence is again a more neutral term that covers more of what is actually happening.
• Array: I agree Array is not a good term either but Array in C/C++/Rust usually have fixed lengths so I thought it was the least bad option with a name that is bearable to type in.

[jacogr]

We are not in C++ - we are coding to SCALE with the de-facto implementation in Rust. I can't even begin to unpack your comments above - it is just so wrong in terms of Substrate.

The naming as you have fought for is utterly confusing. Completely and utterly. (And I'm putting it lightly) As it stands, what is in here has in no way, shape or form any relation to what is in the de-facto standard which is the Substrate frame implementation types.

I'm really, really, putting it lightly. You are trying to build something "generic" and completely missing the mark for the target where we actually use this.

But you are coming from a side where you don't actually use any of this stuff, neither actually have to replace what is in Substrate, neither are you trying to map this naming to something that maps to what is in Substrate. So you are thinking "o, lemme do CS principles" - keep it simple, map to the damn thing we actually want to encode and make is simple. As it stands, this is failing horribly and it utterly confusing since it has just no baseline apart from random CS-like naming.

What you should have done -

• instead of these random names
• you should have mapped to the Rust types

At this point in time, you have a mixed bag, and well, as I said, it just fails to make sense looking at where it comes from.

Anyway, you may not like it, but I also don't like what it in here, and even less now that it is mainstream. So, doing them best working with this baseline to try and make it sensible with at leats renames. (And it fails, but, the base is shaky, although in conveys all info - which is the most important thing, we can encode/decode, so the rest... well, we will make do)

I don't have the concept of a struct in JS, I have objects. Arjan has something different in Python. The Subcan team knows it differently in Ruby - however, we all full-well know and understand what a "struct" is in Rust. It makes sense. A composite, well, it is just another thing that just makes the whole ecosystem even more difficult to navigate and maintain.

As always, YYMV.

EDIT: Sorry for sounding grumpy, but this is now the second time I needed to wade through this "naming mess" and learn yet-another-language to try and make sense to it. I really don't want to maintain this stuff myself for the next 10 years since nobody can get into it since it is just plain weird. But, that is where we are. It misses the mark for the target... completely.

[dvdplm]

I for one don't mind the current naming scheme.

Yes, it took some getting used to, but the effort that Andrew and Robin put in to making the whole library as generic as possible has paid off greatly in other circumstances and I don't think we should rush into Rusty names just because that's what we and many in our current audience is used to.

I agree with @Robbepop that "enum" is a slightly unfortunate name even for Rust itself; switching out "composite" (which I don't love) to "struct" would be terrible imo. "sequence" is as good of a name as it gets.

Substrate requires practicians to learn a metric fuckton of terminology already; I'm not convinced then learning what "composite" and "variant" maps to is going to be a big problem.

[jacogr]

I love what Andrew did here and I am certainly a fan of his work. But we dropped the ball since we try to not map to what we have as input.

All I can say is - actually use the libs and then come back. Andrew has, he logged this issue. I have, I am supporting the direction he wants to take it in. Sadly we are blocked by an idealized view way people who don’t actually code against the stuff.

To each their own. Next time, hopefully we do it right.

You are all complaining about enum, guess what, the people using Substrate and these APIs got used to it. So that is the crux - it is not correct in an idialistic world, but for the one we map, it is perfect.

In case you don’t get it yet - what this does is amazing due to Andrew’s efforts, the way you did the paper design of the interfaces lacks and doesn’t make sense.