This repository has been archived by the owner on Jun 30, 2023. It is now read-only.
Improve argument collection API #74
Labels
enhancement
New feature or request
Comments
kzu
added a commit
that referenced
this issue
Jan 15, 2021
Previously we had code in quite a few places that would get the `ParameterInfo` from the argument collection, and then do something with the (untyped) object value subsequently. Since the collection was not typed, we were forcing boxing of every argument value, which in turn would prevent types that cannot be boxed in the future (such as Span<T>). In addition, the validation of the argument value assignment according to its parameter info was quite loose and had to be accounted in more than one place too. This commit adds a strong-typed `Argument<T>` record class (and its base un-typed class) which is now used consistently. It's also used to validate the type conversions so this is consistent regardless of which other APIs that end up creating argument collections are used. In order to make it easy to create strong-typed argument collections, a bunch of generic factory method overloads were added to make this seamless: * `ArgumentCollection.Create<...>(parameters, args)` * `MethodInvocation.Create<...>` * `IMethodInvocation.CreateResult<...>` and `CreateValueResult<...>` extension methods. The first one is the actual one creating the typed arguments. The others are mostly pass-through to it, and are added as convenience to avoid having to invoke `Argument.Create<...>` explicitly, especially since the `ParameterInfo[]` to use in the collection is always the one in the method invocation itself. Fixes #74.
kzu
added a commit
that referenced
this issue
Jan 15, 2021
Previously we had code in quite a few places that would get the `ParameterInfo` from the argument collection, and then do something with the (untyped) object value subsequently. Since the collection was not typed, we were forcing boxing of every argument value, which in turn would prevent types that cannot be boxed in the future (such as Span<T>). In addition, the validation of the argument value assignment according to its parameter info was quite loose and had to be accounted in more than one place too. This commit adds a strong-typed `Argument<T>` record class (and its base un-typed class) which is now used consistently. It's also used to validate the type conversions so this is consistent regardless of which other APIs that end up creating argument collections are used. In order to make it easy to create strong-typed argument collections, a bunch of generic factory method overloads were added to make this seamless: * `ArgumentCollection.Create<...>(parameters, args)` * `MethodInvocation.Create<...>` * `IMethodInvocation.CreateResult<...>` and `CreateValueResult<...>` extension methods. The first one is the actual one creating the typed arguments. The others are mostly pass-through to it, and are added as convenience to avoid having to invoke `Argument.Create<...>` explicitly, especially since the `ParameterInfo[]` to use in the collection is always the one in the method invocation itself. Fixes #74.
kzu
added a commit
that referenced
this issue
Jan 15, 2021
Previously we had code in quite a few places that would get the `ParameterInfo` from the argument collection, and then do something with the (untyped) object value subsequently. Since the collection was not typed, we were forcing boxing of every argument value, which in turn would prevent types that cannot be boxed in the future (such as Span<T>). In addition, the validation of the argument value assignment according to its parameter info was quite loose and had to be accounted in more than one place too. This commit adds a strong-typed `Argument<T>` record class (and its base un-typed class) which is now used consistently. It's also used to validate the type conversions so this is consistent regardless of which other APIs that end up creating argument collections are used. In order to make it easy to create strong-typed argument collections, a bunch of generic factory method overloads were added to make this seamless: * `ArgumentCollection.Create<...>(parameters, args)` * `MethodInvocation.Create<...>` * `IMethodInvocation.CreateResult<...>` and `CreateValueResult<...>` extension methods. The first one is the actual one creating the typed arguments. The others are mostly pass-through to it, and are added as convenience to avoid having to invoke `Argument.Create<...>` explicitly, especially since the `ParameterInfo[]` to use in the collection is always the one in the method invocation itself. Fixes #74.
kzu
added a commit
that referenced
this issue
Jan 15, 2021
Previously we had code in quite a few places that would get the `ParameterInfo` from the argument collection, and then do something with the (untyped) object value subsequently. Since the collection was not typed, we were forcing boxing of every argument value, which in turn would prevent types that cannot be boxed in the future (such as Span<T>). In addition, the validation of the argument value assignment according to its parameter info was quite loose and had to be accounted in more than one place too. This commit adds a strong-typed `Argument<T>` record class (and its base un-typed class) which is now used consistently. It's also used to validate the type conversions so this is consistent regardless of which other APIs that end up creating argument collections are used. In order to make it easy to create strong-typed argument collections, a bunch of generic factory method overloads were added to make this seamless: * `ArgumentCollection.Create<...>(parameters, args)` * `MethodInvocation.Create<...>` * `IMethodInvocation.CreateResult<...>` and `CreateValueResult<...>` extension methods. The first one is the actual one creating the typed arguments. The others are mostly pass-through to it, and are added as convenience to avoid having to invoke `Argument.Create<...>` explicitly, especially since the `ParameterInfo[]` to use in the collection is always the one in the method invocation itself. Fixes #74.
kzu
added a commit
that referenced
this issue
Jan 15, 2021
Previously we had code in quite a few places that would get the `ParameterInfo` from the argument collection, and then do something with the (untyped) object value subsequently. Since the collection was not typed, we were forcing boxing of every argument value, which in turn would prevent types that cannot be boxed in the future (such as Span<T>). In addition, the validation of the argument value assignment according to its parameter info was quite loose and had to be accounted in more than one place too. This commit adds a strong-typed `Argument<T>` record class (and its base un-typed class) which is now used consistently. It's also used to validate the type conversions so this is consistent regardless of which other APIs that end up creating argument collections are used. In order to make it easy to create strong-typed argument collections, a bunch of generic factory method overloads were added to make this seamless: * `ArgumentCollection.Create<...>(parameters, args)` * `MethodInvocation.Create<...>` * `IMethodInvocation.CreateResult<...>` and `CreateValueResult<...>` extension methods. The first one is the actual one creating the typed arguments. The others are mostly pass-through to it, and are added as convenience to avoid having to invoke `Argument.Create<...>` explicitly, especially since the `ParameterInfo[]` to use in the collection is always the one in the method invocation itself. Fixes #74.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Currently consuming the API is a tad confusing. Enumerating it will yield the
ParameterInfos, rather than the values. Getting all values is tricky (requires aSelectwith aGetValue(name|index)), and setting values while enumerating is also cumbersome (need to keep the index around).The main helpers and members around the
IArgumentCollectionitself are fine, but it seems like we really need to create a first-classArgumentclass (record?) that encapsulates both the value as well as theParameterInfo, since in many non-trivial cases you need both to operate. This also gives as an opportunity to introduce (perhaps) anArgument<T>which would allows us to have box-free value types support (as long as the Get/Set are used with the right type consistently). This would also improve run-time performance significantly.We could also centralize all type checking (conversion too?) in the new type too.
The text was updated successfully, but these errors were encountered: