Fallout-build / Fallout-build/Fallout

Static classes mix fixed values with behaviour — split them and write down the rule

Open
#587 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

### Problem

Static classes across the codebase mix fixed values with behaviour. The name says data, the contents include lookups, file-system probes and I/O. Callers cannot tell from the call site which one they are getting.

`Constants` was the worst case and is the reason this issue exists. It held ~31 `const` values *and* ~19 static methods that walk parent directories, probe `Directory.Exists`, and read `EnvironmentInfo`. Splitting it (#586) took a day of back-and-forth precisely because nobody could answer "can this move?" without reading every member — the values could move to `Fallout.Core`, the behaviour could not.

Others with the same shape, found by scanning for static classes carrying both `const` fields and static methods:

| Class | consts | methods |
|---|---|---|
| `PathConstruction` (`Fallout.Utilities/IO`) | 3 | 25 |
| `ReflectionUtility` (`Fallout.Utilities/Reflection`) | 3 | 16 |
| `Logging` (`Fallout.Build`) | 3 | 12 |
| `EncryptionUtility` (`Fallout.Utilities/Security`) | 10 | 6 |
| `SignPathTasks` (`Fallout.Common/Tools/SignPath`) | 7 | 6 |
| `PlatformNames` (`Fallout.Persistence.Solution`) | 12 | 3 |

Not every one of these is wrong — a utility class of related functions is fine. The problem is that there is no rule, so "static class" is used for constants, for pure functions, and for file-system and network work alike.

The cost shows up whenever a type needs to move between layers. A class of pure values can sit in the innermost project; anything touching I/O cannot. Today that has to be worked out member by member, every time.

### Outcome

A written rule separating the two, and the worst offenders split to match it. Whether a static class can move inward is answerable from its name and its dependencies, not by reading every member.

### Acceptance criteria

- [ ] Convention recorded in [docs/agents/conventions.md](../blob/main/docs/agents/conventions.md): a static class holds **either** fixed values **or** behaviour, not both.
- [ ] The convention says how to name each kind, so the distinction is visible at the call site.
- [ ] The classes in the table above are assessed against it, and the ones that genuinely mix concerns are split.
- [ ] Behaviour that reads the file system, the environment, or the network is not reachable from a type whose name suggests it is data.
- [ ] No behaviour change. These are moves.

### Notes

- #586 already did this for `Constants` → `Fallout.Core.Constants` (values) + `Fallout.Build.Shared.FalloutPaths` (behaviour). Use it as the worked example.
- Distinct from the de-statification work, which is about making statics injectable. This is narrower: it is about statics that lie about what they contain. A class can be honestly named and still be static.
- Related: #584 (centralising configuration values), #310 (FT-5, context-scope resolvers and process defaults).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.