[API Proposal]: Expose Reflector.For directly, and cache the Members array

[dennisdoomen]

Background and motivation

Reflector is the class that does the real work: it walks the hierarchy, applies the visibility rules, resolves explicitly implemented and default interface properties, and lazily caches the results. It is reachable only through a private static method:

private static Reflector GetFor(Type typeToReflect, MemberKind kind)
{
    return ReflectorCache.GetOrAdd((typeToReflect, kind),
        static key => new Reflector(key.Type, key.Kind));
}

Every public entry point goes through it:

public static PropertyInfo[] GetProperties(this Type type, MemberKind kind) => GetFor(type, kind).Properties;
public static FieldInfo[] GetFields(this Type type, MemberKind kind) => GetFor(type, kind).Fields;
public static MemberInfo[] GetMembers(this Type type, MemberKind kind) => GetFor(type, kind).Members;

Two things follow.

A dictionary lookup per call. Callers that ask for properties, then fields, then members for the same type pay three ConcurrentDictionary lookups on a tuple key, including the tuple's GetHashCode over a Type and an enum. In a hot loop over a large object graph, that is measurable, and it is avoidable if the caller can hold the Reflector itself.

Members is not cached. Unlike Properties and Fields, the Members property allocates a new array on every single call:

public MemberInfo[] Members => [.. Properties, .. Fields];

So GetMembers is the one entry point with no caching benefit at all, which is easy to miss given the other two are carefully double-checked-locked. Exposing Reflector makes this visible; it should probably be cached regardless of whether the type is exposed.

Exposing the type would also give callers a handle they can pass around, which reads better than threading (Type, MemberKind) pairs through their own code.

API Proposal

internal sealed class Reflector
{
    public static Reflector For(Type type, MemberKind kind);

    public PropertyInfo[] Properties { get; }

    public FieldInfo[] Fields { get; }

    public MemberInfo[] Members { get; }
}

API Usage

var reflector = Reflector.For(typeof(Order), MemberKind.Public | MemberKind.Internal);

foreach (var property in reflector.Properties)
{
    // ...
}

foreach (var field in reflector.Fields)
{
    // ... no repeated cache lookup
}

Alternative Designs

• Keep Reflector hidden and just fix the Members caching. Solves the correctness half of the problem with no new surface, and is worth doing either way.
• Expose it but keep the constructor private, forcing everything through For so the cache is never bypassed. That is what the proposal above does, and it seems clearly right: a public constructor would let callers create uncached instances by accident.
• Return an interface rather than the concrete class, to keep implementation freedom. Probably over-engineering for a type this small.

Risks

• Since everything in the package is emitted as internal, "public" here means visible within the consumer's assembly. So this is a low-risk change in terms of binary compatibility, but it does commit the library to Reflector as a named concept in the API.
• Caching Members changes the identity of the returned array from "fresh each call" to "shared". If any consumer is mutating the returned array in place, that would now corrupt the cache. The existing Properties and Fields already have this exposure, so it is consistent, but it is worth a documented remark that returned arrays must not be modified.

Are you willing to help with a proof-of-concept (as PR in that or a separate repo) first and as pull-request later on?

No