Skip to content

Update AV1125: Don't expose stateful objects through static members#353

Open
dennisdoomen wants to merge 1 commit intodevelopfrom
copilot/pr298-update-av1125
Open

Update AV1125: Don't expose stateful objects through static members#353
dennisdoomen wants to merge 1 commit intodevelopfrom
copilot/pr298-update-av1125

Conversation

@dennisdoomen
Copy link
Copy Markdown
Owner

This PR updates guideline AV1125.

It was split out of #298 so the change can be reviewed independently.

Files:

  • _rules/1125.md

Part of the replacement for #298.

Split from #298.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A stateful object is an object that contains many properties and lots of behavior behind it. If you expose such an object through a static property or method of some other object, it will be very difficult to refactor or unit test a class that relies on such a stateful object. In general, introducing a construct like that is a great example of violating many of the guidelines of this chapter.

A classic example of this is the `HttpContext.Current` property, part of ASP.NET. Many see the `HttpContext` class as a source of a lot of ugly code.
A classic example of this is the `HttpContext.Current` property, part of classic ASP.NET. Many see this as a source of a lot of ugly code, because it hides an implicit dependency on a global state object. Instead, make dependencies explicit by injecting them through a constructor or method parameter.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to refer to classic ASP.NET; there are plenty of cases in modern .NET that are still a pain when writing tests:

  • System.Environment.MachineName
  • System.Environment.GetEnvironmentVariable (can't run tests in parallel that depend on it)
  • System.DateTime.UtcNow
  • System.IO.Path.Combine (depends on OS-level directory separator)
  • System.IO.Path.GetInvalidFileNameChars
  • System.IO.File.Exists

The rule title could be changed to:
Don't expose global state through static members

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants