Make GetPropertyValue generic to avoid boxing

[AntonC9018]

Is your feature request related to a problem? Please describe.

GetPropertyValue method from IValidationRule<T> boxes the value, which leads to a bunch of unnecessary allocations.

Describe the solution you'd like

Add an overload that has a generic parameter for the expected property type. Conceptually like this, but you may tweak it like say if you wanted to throw if the type doesn't match, or have special handling for value types:

TProperty? IValidationRule<T>.GetPropertyValue<TProperty>(T instance) => PropertyFunc(instance) as TProperty;

Describe alternatives you've considered

No response

Additional Context

No response

[JeremySkinner]

Hi

The IValidationRule<T>. GetPropertyValue method isn't actually used within the FluentValidation codebase, so from an internal perspective this has no effect on allocations. The purpose of this method is for external consumers who need to build metadata specifically in the case where the property value type is unknown at runtime, which is why it returns object. Perhaps you could elaborate more on your use case for needing it to be generic?

[AntonC9018]

Currently #1688 (comment) is basically my use case.

[JeremySkinner]

I don't think that use case mandates a generic here. Have you done a performance analysis to see how much of a difference that would actually make within your codebase?

[AntonC9018]

Well, it would save a boxing allocation for each time it's called, so it's just a small win. I know it's tiny and you'd tell me not to care, but I don't see why not cut down on a few allocations if the solution is so easy.

[JeremySkinner]

I'm aware that it saves a boxing allocation, but generally within a codebase there are more important performance issues to worry about, and something like this is not likely to be a contributor to performance issues unless its on a hot-path within your codebase (which was why I was asking if you'd done a performance analysis). We've done a lot of work over the years to optimise the codebase and reduce allocations where it was appropriate to do so, but I don't feel that this is one of them.

This also isn't a quick win as it introduces some complexity and ambiguity. With this method a null has 2 very different meanings - if the value of that method returns null, is that because the property value is null, or is it because the developer specified the wrong type within the generic? The result is unreliable, which is why I'm not at all keen on this. To handle this properly would require a more complex solution which I don't feel is worth it.

[JeremySkinner]

Would a TryGetPropertyValue work for you?

bool TryGetPropertyValue<TProperty>(T instance, out TProperty value)

I think this is probably the simplest way to correctly handle if the type specified is incorrect without resorting to dual meaning of null, or runtime exceptions. If the property isn't of type TProperty then the method would return false.

As this would require a new method on the interface this is technically a breaking change, so wouldn't be something we could implement until the next major release.

[AntonC9018]

Sure, thanks