Rename member
Opened this issue · 5 comments
slicec makes extensive use of the term 'member' that has no meaning in Slice. We need to rename all these members.
The correct terms are fields and parameters.
For context, slicec
indeed has a Field
struct, and a Parameter
struct, that see much use.
But, these structs are very similar, since fields and parameters are very similar in Slice.
Very often, we'd write two functions, one for each struct, and the implementations would be identical,
so to avoid code duplication, we decided it'd be better to introduce an umbrella type.
So we added the Member
trait, which is implemented by Field
and Parameter
.
It's sole purpose to reduce code duplication in cases where these types have identical behavior/implementations.
Member
was simply the closest umbrella term for these two things.
You're correct Member
isn't a Slice term, it's out own internal compiler jargon and we should be careful to never leak it.
To my knowledge, we don't. If you find somewhere where we do, please fix it or open an issue.
And maybe there are some places where we take a Member
, and we actually only need it for one of either Field
or Parameter
.
These places should be updated to use the concrete types, for sure.
But for implementations that are shared between Field
and Parameter
, we can't just replace Member
with them, without duplicating the function.
We should consider merging Field and Parameter into a single struct (Field). Then no need for this umbrella trait with an odd name.
Just as an aside, having lots of types is actually very standard in Rust. Even those that are nearly identical to one another.
Rust is all about specificity and safety. It's also about offloading as much checking as possible to the type-system.
Having a struct where sometimes this field shouldn't be touched, and other times it must be used is a Rust smell.
Also, in my opinion, just comparing the structs isn't a good indicator of actual similarity.
Class
and Exception
are almost identical, even more so than Field
and Parameter
.
Result
and Dictionary
are literally identical structs (they both just hold two types).
Looking through the code, I think this would be a net-zero change. Some things would get simpler, others more complicated.
I'd actually considered this in the past, but while they are very similar, there are some important differences:
- parameters can be streamed, fields cannot. Seeing
is_streamed
on fields would looks odd (since streamed fields aren't a thing), and indeed, this field would be dead code for certain cases. - fields support doc comments, parameters don't. So the
comment
field would be dead code for certain cases.
The logic for generating doc comments for them is very different too (if you look atslicec-cs
). - Fields always need to have a name, parameters do not (single return types).
- Fields and parameters support a different set of attributes (readonly, deprecated, etc.), and the attribute checker just matches on type (a very simple check). We'd have to look at the parent type, since type alone isn't sufficient, and looking at the parent type is also harder because:
- they have different parent types. Calling
parameter.parent()
gives an&Operation
. Merging Parameter and Field would mean this could no longer return a concrete type, which makes the API less convenient for consumers (they'd get a trait type:&dyn Container<Field>
). - Everywhere where we match the possible
&dyn Container<Field>
s would need to be updated with an extraOperation
case. This case would almost always be dead code. SinceOperations
are very different than structs, classes, exceptions. - fields can cause cycles, parameters cannot. The cycle checker uses this fact to know when to stop checking a branch.
We'd need to re-evaluate the logic it's using to make sure it doesn't hit false positives. At the very least, it would be needlessly checking more things than necessary. - The way you generate links to them is very different in C#, and probably other languages too. Right now we use a simple match
parameter -> paramref
andfield -> see cref
. This is no longer sufficient, we'd need to check the parent, which again, is more difficult now.
I prefer to leave these types separated. It's more semantically cleaner.
At the Slice2 encoding level, parameters are just like the fields of an anonymous struct that gets encoded into a request / response payload.
The differences between struct fields and parameters are very small:
- not having a name at all is unique to return parameter (when there is only one) - not a big difference
- "stream" is unique to parameters
and that's it.
Class and Exception are almost identical, even more so than Field and Parameter.
Result and Dictionary are literally identical structs (they both just hold two types).
This is a bogus argument. The syntax is similar but the semantics is completely different. That's totally different for fields and parameters.