Source
r/csharp•
SentimentalBear

If static classes are bad, what should I do instead?

Stack overflow is generally of the opinion that static classes/methods are bad. I understand the benefits around maintainability, flexibility, and testing.

But then, what to do instead? If I have a bunch of helper methods inside an instantiable class, when should I create it / how many of them should I create? I could for example stick it into a static variable, either in each of the individual classes that use it, or some general class they can access. I suspect that still breaks OOP principles. But then what? Instantiate the helper class every time one of its methods get used? What if that is really often, and happens across different classes?

I'm doing games programming if that makes a difference - I doubt this specific case is going to break performance, but games tend to be much more entangled across classes/layers, and it feels really inefficient to instantiate classes every time you want to call a commonly-used, class-independent method, so I'd like some help understanding the best approach.

Posted May 17, 2023, 11:09 AM UTC

2777
Alikont•

Static state is bad.

Static method that is pure (output is dependent on input only) is ok.

200
SentimentalBearOP•

That is my current principle, but also what the most upvoted answers in Stackoverflow argue against.

14
Epicguru•

Stack overflow likes to disagree with everything and half the people commenting are giving their opinion which they formed at college 30 years ago.

40
roughstylez•

I mean... Just like this sub.

Just like any public congregation of programmers.

How does the saying go "ask 10 programmers their opinion, you will get 11 different answers".

26
dmstrat•

only 11, we must be getting better =)

9
SideburnsOfDoom•

"ask two programmers their opinion, you will get three different answers".

Got it.

5
[deleted]•

[deleted]

1
Alikont•

They argue against static methods with side effects. All those arguments except "parameter creep" are related to side effects. (edit: also it's trully easier to use some oop patterns around non-static methods, like decorators, but in practice you either know that you will use them or not).

Your static methods should not write/read files, access DB, make network requests, etc.

39
dmstrat•

yeah, static methods with side effects is bad in my opinion. That's probably why this topic would be hard to answer: too generic.

7
SentimentalBearOP•

static methods with side effects

This really helps me to frame my thinking on this. I do think it's also about testing, as you can only create test stubs if you have an interface to work with, but I guess there are ways to get around this, like decorators as you say.

4
lordosthyvel•

A pure method won’t need a test stub. Just test the method directly

18
[deleted]•

[deleted]

1
Slypenslyde•

Here's a tough thing about programming.

Sometimes there is an opinion of the crowd that is wrong. Often it's wrong because there is nuance to the answer that is not being discussed. One downside of StackOverflow that quickly became apparent is it rewards SHORT answers, not DETAILED answers. Answering quickly makes it more likely you get the coveted "accepted answer", which puts your answer at the top and means more upvotes. There are some other weird things about SO that lead to this but I digress.

The main thing is to make sure you understand why you are making a static method and when it's not a great idea. It's not just about static state, there are also maintainability and testability concerns. I've seen people make static methods for things that touch "volatile" resources like the file system or network resources. If you do that, you won't be able to make a class using that method use some kind of stubbed or mocked method when you try to test it. Static methods can't participate in polymorphism or IoC so once you use one it's set in stone.

Sometimes we have a method like Math.Sin(). It doesn't have anything "volatile". For any possible input, I have ways to verify if its output is correct. Because of that it's hard to imagine when I'd use a fake or mocked version instead of the real thing. This is a case where the concerns above are addressed and a static method is appropriate.

Then there are methods like File.ReadAllText(). That breaks the rule. I don't think Microsoft made a poor decision making this a static method, but because file I/O is something I want to fake or mock in my unit tests it means I usually have to make some IFileSystem abstraction (or use a package with one) so I'm able to use instance methods that CALL it and can be faked or mocked. Not so in my smaller scripts and prototypes, but I don't hold those to the same standards as my professional code. I still think Microsoft was right, but I do wish a filesystem abstraction was not a third-party thing.

So it's all case-by-case. You have to think about whether a method should be static, and you have to ask yourself if the downsides will cause problems. For a lot of methods, they won't. If you've thought about it and feel like it's OK, then do it. Be honest with yourself later if it does cause problems and switch it to instance if so.

Even when you're completely right about it, there are still some people who will criticize the decision. I personally almost never make methods static. I think the loss of the capability to use IoC is too heavy a price to pay. But I also acknowledge that's an extreme position. So if someone has otherwise good code and my only problem is their use of static methods, I keep it to myself until they ask me a question I can legitimately say that static methods can't answer.

Put another way, most times we say to never do something, we know there is some small % case where it's perfectly fine and maybe even advantageous. "Never do this" is a silly shortcut we take to the idea that it takes a lot of knowledge and intuition newbies and novices do not have to be right about when to do it. So the experts say "never do it" to reflect the idea that getting something wrong has heavy costs that may not be apparent until very late.

That's not a license to break every rule. It's a reminder to think HARD about everything, even simple decisions, when writing large-scale code or code you want to maintain for a long time. Sometimes that cognitive burden's so great we decide to abandon one of our tools so we can focus on a smaller set of patterns and practices and use familiarity to help us manage complexity.

33
Alikont•

I don't think Microsoft made a poor decision making this a static method

I think they made a bad decision.

It'll be better to have something like FileSystem.Current.ReadAllText(...), where FileSystem.Current returns IFileSystem abstraction that can be mocked. They kind of did that with WinRT filesystem (interfaces and all that).

The same goes for clock. Just now in .NET 8 they introduce clock abstraction, and asp.net core has at least 2 of them. This means that they realized and admitted the mistake.

Instead of DateTime.UtcNow it would make sense to have SystemClock.Default.UtcNow.

14
Slypenslyde•

That's not a bad opinion.

I think on a 3-point scale with "bad", "OK", and "good" what they did is "OK". More often than not I find there's some value in having an abstraction more domain-specific than just a wrapper around their methods. So I'm usually not abstracting "the file system" but "a repository-like object" and that means the concrete object can freely use the static methods.

But it wouldn't hurt if they had a more full-fledged abstraction around their filesystem access and I always do wish they'd make a new one.

13
emn13•

Abstractions aren't free; they cause mental load. Adding a seemingly pointless abstraction that has no plausible use cause other than being replaced by a fake implementation for testing purposes doesn't seem hugely convincing.

In particularly, it helps testability only to a very limited extent - after all, if you want to refactor the code to allow replacing the text read by ReadAllText, you can trivially do so - if it's your code. If you can't alter the code; sometimes a more complex integration test with real file-system access is worth it (esp. since file systems are notoriously complex and have real-world tricky interactions your test might miss). Sometimes you want a "real" filesystem that is itself a fake meant for testing purposes (i.e. a RAMdisk, for instance?).

In general, I don't have great experiences with APIs that try to enable replacing hyper-complex dependencies in an attempt to allow testing. However, APIs that allow you capture the effects of that dependency and then depend on some simpler intermediate representation are great. And that's probably what you should be doing here, too: the intermediate representation here simply being string.

However, there's clearly nuance here - I'm sure you're right there are examples where being able to replace the file system for testing purposes would be useful, I'm just not so sure it's a net win. If the aim really is testability, then perhaps a more useful fundamental concept would be some built-in support in the core language itself to (potentially inefficiently) truly redefine actually static things in some context; i.e. have your statics and still have mocking.

In any case, whether the file system is the best example or not, there are certainly quite a few examples of APIs with poorly testable and poorly extensible designs. At least a few apis that take "mode" style enum parameters for instance surely must have had some way to replace those with a proper interface - and for some of those, this wouldn't even have made the syntax from the consumers perspective any different whatsoever; they can still have their well-known static implementations, without preventing alternatives.

And then theres all those APIs that are needlessly huge or messily complex. In theory you could make your own implementation or provide some mock, but in practice you'd need to subclass some monstrosity with dozens of methods, poorly hidden mutable state, many overloads, etc. I'd much rather have a clean, small API that's then helpfully extended with static utility methods that cannot tweak the internals and would equally apply to any replacement, than a huge interface / non-sealed base type with tons of hard-to-fully-understand let along really control corner cases. If you want to improve testability, that is where the BCL really fell down. FileInfo has 55 public methods (counting overloads and property accessors). Stream has 54 - that's just crazy; and fully 38 are non-sealed.

Newer APIs do seem to be better in this regard; for instance the PipeWriter has "just" 15 virtual methods.

I wish most things were more like, say IEnumerable - a relatively small abstraction, with most of the complexity provided by transparent utilities around that abstraction. That would help me a lot more than worrying about a few global statics that I can easily replace with a parameter in the very odd case I care to.

3
psymunn•

Great answer. I definitely have been liking how composition with shared instances of classes has a lot of the benefits of static classes, without sacrificing testability.

It's also worth noting that somethings have to be static, like extension methods, and they often match the litmus for when a static helper method is okay.

3
SentimentalBearOP•

The main thing is to make sure you understand why you are making a static method and when it's not a great idea. It's not just about static state, there are also maintainability and testability concerns.

I think these are the crucial points for me. I just need to think it through, and it's more than just "no state = static good", or "static always bad", but in particular, whether the method is a good candidate to be set in stone, and won't need ever need variants, affect the state of something non-internal to it, or need some type of state adding.

2
angrathias•

I guess they’d hate linq methods then 🤷🏼‍♂️

5
robthablob•

LINQ methods are extension methods, while static, they only depend on their input, so they are effectively pure functions without side effects.

3
angrathias•

Yes I understand how they work, it was a tongue in cheek jab at the dislike of statics in general

3
creamyhorror•

The answers on this post seem better explained and more detailed than the majority on that StackOverflow page. And in fact, it's mostly the top-voted answer that comes out somewhat strongly against static classes. The subsequent ones are less opposed.

StackOverflow is just programmers upvoting answers that sound good, but the quality/detail/insight of which may not actually be that good.

2
solohelion•

It’s largely about testability, and when it’s not, it’s about testability as a proxy for code that’s maintainable. A static method that is a pure function that takes input, does something deterministic, and produces output (and of course has no implicit state) is a fine static method. But a whole bunch of interdependent static methods that can be referenced outside the static class? No please. That’s where it’s useful to have classes that implement interfaces that describe specific behaviors and hide the details from the reader and from consumers of the code.

Sometimes there’s no “good” place to put a helper because the existence of it is the problem in the first place. The other risk is that a few static methods here and there turns into a norm when Junior engineers get in there and follow the pattern that they know the best.

2
midri•

Yup!

Math.Sin = good static implementation.

DateTime.Now = bad static implementation.

3
venomiz•

Exactly this.

1†
warpedgeoid•

There’s nothing wrong with static classes and methods, if that’s what’s is required for an elegant implementation. Absolutism is bad.

27
Canthros•

Absolutism is bad.

Most of the time.

27
scrapmetal134•

I'll echo that static or shared global state is what is considered bad. Static methods are perfectly fine if they don't modify any global state.

I would recommend using Extension Methods where appropriate though as it allows you to attach the helper methods to the class they work on.

22
SentimentalBearOP•

Microsoft's guidance says you should only use extension methods when working with objects in libraries you don't control. That isn't the case for me here, as it's all my own source. The one case that it made sense to me in the past to use it is in creating methods on interfaces, but even that is now in doubt with the introduction of default interface methods. Or is that your point, just that this is a valid use case of static classes/methods in general? (within the framework of the MS guidance)

3
scrapmetal134•

Well yes, because if you owned the library, you could just add the method.

My point was only to show that it existed in the case that you had a static method that made more sense as an extension. Such as operations that need to work on string, or something engine specific like a Vector2 implementation.

2
[deleted]•

what's great about this advice is that MS doesn't even follow it. Once you start digging into the libraries under Microsoft.Extensions.* huge chunks of the code base are extension methods. I joke with colleagues that .NET Core and beyond is just extension methods all the way down.

here is my view, for a lot of things inheritance is rarely the solution especially when it comes to logical boundaries like services. services is where extension methods really shine. you can define an interface with core functionality and then build extension methods off of that core functionality. why? because anything that implements the target interface of the extension methods gets all the extension methods for free without abstract or base classes or find workarounds do something like multiple inheritance nor do I need to add them to the interface as methods to implement. A very good example of this the extension methods off of IMemoryCache and IDistributedCache and anything related to IServiceProvider, IServiceCollection, and/or IConfiguration.

anytime you have something titled <thing name>Helper.cs it's a sign post that those should likely be extension methods.

2
jingois•

meh. it's also a useful tool for separation of concerns.

2
tankpush•

Static classes are not always bad. Simple methods that don't use state (String.IsNullOrEmpty) are totally Ok.

But in many other cases you might want to depend on an interface and not the class itself. And create the instances of all the objects as close to the main (composition root) as possible. It's called dependency injection, and there is a lot of reading material on that.

E.g. https://www.c-sharpcorner.com/UploadFile/85ed7a/dependency-injection-in-C-Sharp/

There are also a lot of libraries that can help do that (autofac).

But I have no experience in game programming so maybe that's not the best practice in this case

15
SideburnsOfDoom•

Simple methods that don't use state (String.IsNullOrEmpty) are totally Ok.

yes, this. Suppose you're written your own static or extension method that does e.g. string.IsXorY.

Ask yourself, can you easily unit test it. e.g.

var actual = string.IsXorY("X");
Assert.IsTrue(actual);

If this is trivial (the function is "pure", stateless, depends only on input) then this is a good candidate for a static or extension method. And keep some tests.

If this is hard, if you're thinking "Well, I could try and insert a mock database and populate it appropriately to get it passing" then it isn't, make it a class with state or service dependencies.

8
chucker23n•

Simple methods that don't use state (String.IsNullOrEmpty) are totally Ok.

Wellllll. IMHO, that method has always been a bit of an ugly hack. In more modern C# times, perhaps they would've left it out in favor of

  1. an extension method (those didn't exist in C# 1.0, and they can be called on null) someString.IsNullOrEmpty()
  2. the null-conditional operator (someString?.Length == 0)
  3. a pattern such as if (someString is { Length : 0 })
  4. nothing: starting with C# 8, if your string can be either null or empty, there's probably something wonky in your API design, and compiler warnings now will narrow that place down for you

Other place where .NET has a bunch of static methods are System.Math and System.Console. Again, I'd argue those aren't necessarily great designs. Console might get an overhaul that is no longer static.

Factory methods are perhaps one of the more common examples where static works.

3
Far_Swordfish5729•

Static state is fine as long as you know what that means and use it when it’s appropriate. It means there’s one copy in memory on that server. It’s a singleton pattern - a very simple one. static readonly reference collections that aren’t modified after creation are a pretty normal low stakes optimization. You don’t need multiple copies and don’t necessarily need the headache of a more complicated solution.

What static lacks is mockability, cross server distribution in farms, and durability across process restarts. Because of that, you’ll tend to escalate to IOC singleton access of an instance class (the IOC container will only make one copy), a Reddis cache server, or actual persistence.

6
maxinstuff•

Methods (which don’t rely on instance specific data) can be static within a non-static class.

6
[deleted]•

I don’t know where the idea comes from that static methods are bad. Try functional programming in C# without them and you will struggle.

6
Canthros•

They're hard to test around, especially if they have side effects, because you can't inject or mock them. That's really a non-issue if the method is pure and all, but messy when it provides access to something external to your app (e. g. file system).

An example from my personal experience was a former coworker put together a logging API that wrapped up a couple different things in a performant way ... that was exposed as an exclusively static interface. Fine in prod, but it meant that people's unit tests regularly spammed emails when the test failed during normal development. Trivially solvable if the logger had been exposed as something injectable, and the first thing I usually found myself doing was writing a wrapper to handle that in whatever project I was standing up.

1
[deleted]•

Sounds like an inexperience problem. People who write unit tests should be mocking all external calls that aren’t part of that function.

1
Canthros•

You work with the people you've got, not the people you want. Also, you should design APIs to make it easy to do the right thing, and hard to do the wrong thing. "Static classes are bad" is the sort of advice or guideline you give to someone who doesn't understand the ins and outs.

1
[deleted]•

I think “undocumented side effects” is the advice. You can write bad code in a regular class also.

0
[deleted]•

Just. No. It defies logic to do this unless there is a network call involved.

0
[deleted]•

Not really, unit testing is about testing that unit of work. Mocking things that could change is about forcing some behavior that’s hard to replicate to ensure the function under test behaves as intended in that failure mode. It isn’t about network calls. Though that is one good reason to mock something, it’s not the only reason.

1
See_Bee10•

I'd opt for injected interfaces as it will tend to reduce coupling, but if someone says X feature of a language is bad recognize it as an opinion. If you ask 10 devs a question you'll get 11 opinions.

5
roughstylez•

I'm doing games programming if that makes a difference - I doubt this specific case is going to break performance, but games tend to be much more entangled across classes/layers

My first reaction was to disagree with this, because usually games have very flat hierarchies a la "everything is an entity or component". But... Neither of us is wrong. Both statements are true, it really just comes down to from which perspective you look at it. However, I would argue the complexity in games comes less from the structure of (code) layers and how code calls other code itself, but more from the many many references that all these entities have with each other.

it feels really inefficient to instantiate classes every time you want to call a commonly-used, class-independent method, so I'd like some help understanding the best approach.

It really comes down to what exactly you're doing. That's kinda part of the problem, there are a lot of things that you can make work by just uing something that's globally available. But if you try to solve these things in a non-global way, it forces you to think really well about how these things actually work.

What you come up with then will also be very different for all your different use cases - vs the unified solution "just put it in a global method". This makes it impossible to give good generic advice about what to do instead, like you ask.

Generic advice is impossible, so let's look at something concrete. Input handling in Unity. In a script, you can check the static method Input.GetKeyDown(Keycode.A) and then move an object left by doing transform.Translate(-1,0,0).

InputGetKeyDown() is a static method. It works, no problem. But that was also a super simple script. When you get more serious, you might want to support gamepads. So now you gotta goto all the places where you use input and have to add the functionality there. Some in your movement script, some in your weapon script, etc.

And because all those things are in different places, how do they know if we're in gamepad or keyboard mode? Guess we need another variable that's globally available, even though it only matters to input. Bam there you have the global state that you already understand is bad.

Whereas, if you had started thinking about how to do this without using global stuff, you'd probably already at least partly considered these things. You might have a class that represents an input-thing, be it gamepad or keyboard, which would be used by these different places.

In that input thing, you can add code for handling this switch between keyboard and gamepad, in a non-global way. And your movement script and weapon script wouldn't have to change, they still listen to a "fire" or "move left" command instead of a concrete "A" key.

But... well you see how that solution for non-global input wouldn't just work the same way for other things which at this moment you still handle in a static way.

4
DangOlHunk•

>Of course, there are exceptions to my dislike of static methods. True utility classes that do not pose any risk to bloat are excellent cases for static methods

From the top rated stack overflow answer. Static classes have utility - they can be overused, or used in the wrong contexts - but I don't think anyone would say to never use them.

3
bigtoaster64•

It's considered "bad", because it can be hard to test and debug. But, a static, I like to call them, "helper" class that has no state, its just there to provide pure features that doesn't impact anything outside of their method calls aka no side effect, is fine. A concrete example of that is an extension class that provides extension methods on a type, like ToString on the Object type.

3
dcabines•

You use a DI container and declare your dependencies in your constructor.

2
SentimentalBearOP•

So every instance will have a reference held in memory to its own personal instance of the helper class? That's less than creating one afresh every time you use it, but this doesn't work great from a memory perspective, where in games you often create many instances of one class.

1
dcabines•

You can set it to be a singleton in the DI container so memory use would be the same.

The difference is your dependencies are explicitly declared in the constructor instead of implicitly via namespaces like with statics.

6
SentimentalBearOP•

Sorry for being thick, but how do you inject it, while simultaneously having it be a singleton held in the class it's being injected into? Something like this?

new DIContainer(DIContainer.DefaultHelper);

I get that saves on the memory of the helpers themselves, but wouldn't it still mean for a pointer reference being stored per instance? (ok it's only 8 bytes per instance on 64 bit, but...)

0
dcabines•

Read through Tutorial: Use dependency injection in .NET and especially look at the Register services for DI section.

services.AddSingleton<IExampleSingletonService, ExampleSingletonService>();

how do you inject it, while simultaneously having it be a singleton held in the class it's being injected into?

You'll want to read up on DI more. When your application starts up it will create the DI container and register everything in it. Then you use it to instantiate your classes and it will take care of providing the dependencies. Your class never has a reference to the DI container itself.

To be clear this isn't anything to do with saving on memory usage and either method will have a negligible impact on memory use. DI is about how you organize your application and is why I expect "Stack overflow is generally of the opinion that static classes/methods are bad." If statics are working for you and they're just helper methods with no state then continue using them. DI is typically for larger services that have their own dependencies, but you can use it to eliminate statics entirely too. It is just another tool for your toolbelt.

1
ziplock9000•

Mate, lots of people will give you a lot of 'best case' or 'hypothetical' do's and don't.

When you're on the production line, that means very little.

Game development due to it's nature especially.

2
BuriedStPatrick•

Static methods (without side effects, also known as "pure") are the simplest to test. There are absolutely cases where static is a much better candidate for your code. Anything that doesn't really on state outside its own method body should be static in my opinion. It will perform marginally better and unit testing doesn't require a ton of setup code or mocking to get your state where it needs to be.

Static classes I only ever use as containers for static methods or constants. If you need a single instance of a class, use the singleton pattern instead. It's implemented in Microsoft's own DI framework, but if you can't use that, it's fairly simple to implement yourself.

Do note that bigger object oriented code bases will mostly rely on non-static code as the state is dynamic and can cover more cases. It's really all about handling that state. The less stateful you have, the more predictable your code will be, the more you can rely on static. The more stateful, the more you will need to abstract things to handle the increased complexity. Once you get the hang of it, you'll start to automatically figure out which to use in certain scenarios.

There's nothing wrong with starting from static, then figuring out you need a stateful service dependency (like accessing a database), then switching to stateful non-static code. And if you can, try to separate your stateful methods into smaller static helper methods where state isn't a factor. Then you can unit test those easily while saving the overall stateful method for your integration tests.

2
chucker23n•

If I have a bunch of helper methods

Consider grouping them by the primary type they operate on, and making them extension methods for that type. That gives you OOP-like semantics, at least.

2
nyklashh•

Inversion of Control - Use a Dependency Container and provide instances for classes that need one.

1
screwcirclejerks•

i mean, you can use them, no one is stopping you. if you're storing a ton of "helper" methods, it can be convenient.

however, they are also static. the moment you need an instance, you'll have to use a normal class.

1
ELichtman•

One thing to add that I haven't seen yet is that static extension methods are a necessary evil that has been, in my opinion, partially replaced.

Extension methods are great for when you have no control over the library and need to extend it. But if you aren't careful you end up building a monstrous infrastructure of "pile of garbage" where everything is spread across not-well-named class files and if you're REALLY not careful you can extend things like Decimal.ToDollarString(). Do you really want every instance of class X to be able to have the option to use your method?

In my opinion, it has been partially replaced. I've become a huge fan of default interface implementations and urge you to look into Nuke.build to see a good example of how they work. After working with nuke and separating targets, parameters, etc into default implementations, I created my own internal framework for our product that changed the behavior of hosting bootstrap based on what IConfigure. So IConfigureRestProtocols will enable my application to have OpenApi, web security, etc. And it's much easier to see the where things live than RestBasedServiceProviderExtensions.

1
cancerouslump•

Now that interfaces can support static abstract methods, I'm finding them particularly useful when you want a virtual constructor that takes parameters. In one project I'm working on right now, I have a deep class hierarchy and need a way to generically create any of them while deserializing from disk. Each object inherits from an "ILoadable" interface that has a static abstract method "Node Load(tokens, index)", and each class can construct itself in whatever way it needs to.

Point being: nothing is bad or good in programming: its (a) useful or not; (b) testable or not; (c) performant or not; and (d) maintainable or not. If you find using a static is useful, testable, performant, and maintainable, go for it!

1
karl713•

Static classes are great for things like helper functions. There's a reason Math is a static class for instance, most of my projects have various "StringHelper" classes and such for things with compartmentalized logic (they don't call other things and everything is deterministic / stateless)

But where statics are bad is you don't want your core app logic like communication, service management, etc in a static for the commonly addressed reasons

1