dennisdoomen / dennisdoomen/reflectify

[Feature]: Document indexer exclusion, caching behaviour and internal visibility in the README

Open Beginner friendly
#167 0 comments 0 reactions 0 assignees View on GitHub
feature triage
Dominant language
C#
Stars
80
Forks
6
Avg merge
1d 12m
Merged PRs (30d)
11

Description

### Background and motivation

The README explains the API well but leaves out three behaviours that will surprise people. All three are things you can only discover by reading the source, which defeats the purpose for a package designed to be dropped into someone else's library.

**1. Indexers are silently excluded from `GetProperties`.**

`OrderedPropertyCollection.Add` skips them outright:

```csharp
if (property.IsIndexer())
{
// We explicitly skip indexers
}
```

This is a deliberate and probably correct decision, but it is invisible. The README documents `GetProperties` and, separately, `FindIndexers`, without ever connecting the two. A reader would reasonably assume `GetProperties(MemberKind.Public)` returns every public property, indexers included. Anyone building a member walker on top of this will quietly lose data and not know why.

**2. Everything is cached forever in a static dictionary.**

`ReflectorCache` is a `static readonly ConcurrentDictionary` keyed on `(Type, MemberKind)` that is never cleared. Great for performance, and there is a `PerformanceSpecs` file protecting it. But the consequences are not documented anywhere: the cache pins `Type` objects and therefore assemblies, and it grows without bound for dynamically generated types. Consumers making a decision about embedding this in their own library deserve to know. (Tracked separately as a potential code change; this issue is about documenting the current behaviour regardless of what is decided there.)

**3. All types are emitted as `internal`.**

Every type in `Reflectify.cs` is `internal`, which is exactly right for a content-only package, since it prevents two consumers from colliding. But the README never says so. The consequence a reader needs to know is that Reflectify types cannot appear in their own public API surface: you cannot expose a `MemberKind` parameter from a public method, and you cannot return `Reflector`. That is a design constraint on anyone building a library on top of it, and it is better learned from the README than from a compiler error.

A fourth, smaller one: the README says the package requires C# 12, and there is a closed issue about `LangVersion`. Worth confirming the stated requirement matches what the props file enforces, so the two do not drift.

### Alternative Concerns

- Add a short "Things to know" or "Behaviour and limitations" section to the README covering all four points. Low effort, high value, no code changes.
- Put the indexer note in the XML documentation on `GetProperties` as well, since that is where someone will actually be looking when they hit it. The doc comment currently says "Gets the public, internal, explicitly implemented and/or default properties of a type hierarchy" with no mention of the exclusion.
- Alternatively, reconsider the behaviour itself and add a `MemberKind.Indexers` flag so callers can opt in. That is an API change and belongs in its own issue, but if it were done, the documentation problem largely goes away.

### Are you willing help with a pull-request?

No

Contributor guide

Open the contributing guide

Research direction

Start with the README and compare its existing API descriptions with OrderedPropertyCollection.Add, ReflectorCache, Reflectify.cs, PerformanceSpecs, the GetProperties XML documentation, and the props file. Add a concise section covering indexer exclusion, permanent caching, internal visibility, and the enforced C# version; update GetProperties documentation if appropriate, and confirm all statements match the source.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
documentation
Issue type
Documentation
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.