refactoring-direction

name	refactoring-direction
description	Refactoring direction rules for Gum. Trigger when proposing or performing refactors that change how code is shaped — extracting helpers, choosing between static and instance, deciding where new logic should live. Applies to all Gum source projects.

Refactoring direction

When refactoring Gum code, always move toward instances, interfaces, and dedicated single-responsibility classes. Never move the other way.

Specific rules

1. Never promote an instance method, field, or class to static. Even if the method "has no instance state right now," keep it as an instance member. Static methods are sticky — they grow callers everywhere, can't be substituted in tests, and can't be evolved without source-breaking every caller. Instance methods preserve future optionality.

2. Never demote an interface to a concrete type. If a parameter or field is currently typed as an interface (ISelectedState, IDialogService, IUndoManager, etc.), keep it that way. Adding a new dependency? Inject it as an interface, not a concrete class.

3. Prefer extracting a new single-responsibility class over adding to a "god class." GraphicalUiElement, CodeGenerator, and similar large classes should not grow. New behavior goes in a new controller/service class that the existing class composes with, even if the new class starts with one method.

   Worked example — composition when there's no shared base class available. CircleRuntime and RectangleRuntime (MonoGameGum/GueDeriving/) each spend their one inheritance slot on a platform base (GraphicalUiElement/SkiaShapeRuntime), so when their XNALIKE-only Gradient/Dropshadow logic turned out to be near-identical, composition was the only option (PR #3427). The extraction used structs, not classes: Clone() relies on MemberwiseClone to copy backing fields by value for free, and a class field would instead let the clone and the original share one mutable instance. The structs also don't cache the renderable slot references they push into — callers pass the current slots on every call — so nothing can go stale when Clone() rebuilds fresh slots. Reach for this shape — composed struct, no cached mutable refs — whenever two sibling classes duplicate logic but can't share a base class and the state is safe to copy by value. This is distinct from gum-cross-platform-unification, which shares one runtime's code across platforms, not between two different runtime classes on the same platform.

4. Prefer constructor injection over *.Self singletons. Don't reverse the tool's migration off static singletons.

   • ObjectFinder.Self is the sanctioned exception — fine in new or refactored code; it won't be removed. It is intentionally still a static singleton across the entire codebase (tool, runtimes, tests); an IObjectFinder interface exists and is DI-registered (Builder.cs binds it to the ObjectFinder.Self instance), but the ~476 .Self call sites are intentionally left as-is — replacing them wholesale is a project-wide refactor, not a drive-by cleanup. Calls like ObjectFinder.Self.GetBaseElements(...) / ObjectFinder.Self.GetDefaultChildName(...) are acceptable in any context. No tool singletons should be reintroduced.
   • RenderingLibrary.* / InputLibrary.* runtime singletons are also sanctioned exceptions — out of scope in any phase. Renderer.Self, SpriteManager.Self, ShapeManager.Self, LoaderManager.Self, Cursor.Self, Keyboard.Self, etc. are shared across all runtimes (not tool-only), so draining them is a runtime-wide refactor outside the tool's decoupling goal — leave them as statics, like ObjectFinder.Self. The drain line is ownership: tool-owned singletons (SelectedState, PluginManager, …) are in scope; runtime-owned ones are not.
   • Ownership isn't decided by which folder a class's .cs file sits in — check whether Gum.csproj excludes it (<Compile Remove>) in favor of a link into GumCommon.csproj, the same treatment as ObjectFinder.cs; that makes it a shared, headless (net8.0, no WPF) singleton already reachable from every runtime backend and Gum.Presentation, an ObjectFinder-class exception rather than a drain target. StandardElementsManager (Gum/Managers/) is exactly this case.
   • Plugins are MEF parts and drain the same as core classes — in scope for Phase 2. Inject either via an [ImportingConstructor] param or a public settable property (satisfied by an ; live example: 's ) — both are real DI. The axis that matters is , not ctor-vs-property. MEF can only inject types bridged into its container in via . bridge once in (), then the plugin injects . Use explicit typed per-service bridges, not a generic (over-engineering at this scale). A plugin taking / itself as a ctor param is a construction cycle, despite looking like the plugin host injecting itself into its own plugin: is DI-constructed with an empty ctor () and is fully built ever composes a plugin, so in bridges the same already-built singleton a call would have fetched — it drains like any other service. The one constraint: the consuming ctor must not call a method on the injected synchronously during construction — MEF populates 's own loaded-plugin list via all plugins (including the consumer) are constructed, so a same-ctor call touching that list would see it incomplete. Storing the reference for later use (, event handlers) is always safe.

When testability is the driver

If the reason you're tempted to make something static is "so a test can call it without constructing the owner," the right answer is the opposite: keep it instance, and either (a) construct the owner in the test with stub dependencies, or (b) extract the logic into a new small class that's cheap to construct. The test's friction is a signal that the owner has too many responsibilities, not that the method should be static.

An extraction-for-testability is not done until the extracted unit has a test. When you pull logic into a new class/service/ViewModel/utility specifically to make it testable, add the test before considering the refactor complete — test-first if the move also changes behavior, or a characterization (pinning) test capturing current behavior if the move preserves it. Do not let the tdd skill's refactor/rename exemption talk you out of it: extracting a new unit is not a pure rename, and an extracted-but-untested seam wastes the entire point of the extraction.

Drains are usually behavior-preserving — calibrate the test to that. Most singleton/Locator drains change only how a dependency is obtained (static Locator → injected field), not what the code does, so test-first TDD has nothing to specify — the fitting tool is a characterization (pinning) test. But the compiler and DI container already verify most of a drain: injection wiring is type-checked, construction cycles throw at startup, and a static→instance conversion makes the compiler flag every stale call site. So a pinning test's marginal value is modest — add one where the injected seam is clean and the assertion is meaningful, and skip it where the harness would have to contort (e.g. hand-rolling a delegate to mock an out parameter) in favor of the compiler plus a manual check. The drain's real payoff is enabling later behavior tests, not the test written during the drain itself. This does not loosen the extraction rule above: pulling out a new unit still requires its own test — a static→instance conversion is not an extraction.

Before refactoring across runtimes

If a refactor touches shared runtime/rendering code (GumCommon, RenderingLibrary, anything under Runtimes/, or MonoGameGum), read gum-runtime-topology first. The same source is compiled into many assemblies and into FlatRedBall via shared projects, so "builds clean in AllLibraries.sln" does not mean "didn't break a consumer" (the WPF runtime and FRB are not in that solution).

Converging per-platform duplicate files

A recurring Gum refactor is driving two (or more) per-platform copies of the same file toward byte-for-byte identical content, so they can eventually collapse to one #if-gated linked source. This is done incrementally — block by block, mirroring #if RAYLIB / #if !RAYLIB in both copies wherever a difference is genuinely platform-specific, so the cross-file diff shrinks toward empty. If you're touching code that exists as duplicated per-platform copies (e.g. CustomSetPropertyOnRenderable.cs, GueDeriving/*Runtime.cs), read gum-cross-platform-unification for the full technique before editing.

Why this matters in Gum

Gum's tool code is mid-migration from static singletons (PluginManager.Self, *Manager.Self, etc.) to constructor-injected services. Every new static is a step backward and undoes that migration's payoff. The runtime libraries are similar: GraphicalUiElement is already bloated, and the project memory explicitly calls out "avoid adding new properties/methods directly to this class — prefer separate controller/manager classes." The direction in this skill is the same direction the rest of the codebase is moving.