Open bernardnormier opened 9 months ago
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:
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.comment
field would be dead code for certain cases.
The logic for generating doc comments for them is very different too (if you look at slicec-cs
).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>
).&dyn Container<Field>
s would need to be updated with an extra Operation
case. This case would almost always be dead code. Since Operations
are very different than structs, classes, exceptions.parameter -> paramref
and field -> 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:
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.
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.