dennisdoomen / dennisdoomen/reflectify

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

Open
#166 0 comments 0 reactions 0 assignees View on GitHub
api-suggestion
Dominant language
C#
Stars
80
Forks
6
Avg merge
1d 12m
Merged PRs (30d)
11

Description

### 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:

```csharp
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:

```csharp
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:

```csharp
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

```C#
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

```C#
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

Contributor guide

Open the contributing guide

Research direction

Start at the private GetFor method and the Reflector Properties, Fields, and Members accessors described in the issue. Trace the existing cache behavior before evaluating the proposed For entry point and Members caching. Done means the API proposal is implemented without bypassing the cache and existing reflection behavior remains covered by the project's tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.