8

When reading about when to make a method static or not, I've seen a general principle, as summarized by this post, that a method should only be static if it does not modify a state and its result depends only on the parameters provided to it. However, the most highly voted answer in this post states that static methods should be used whenever possible. Many of the answers in this post say that one should do whatever is most logical.

In my case, I have ~15 methods in a common utility class because they are used in multiple places and don't make sense as a member of any model or view model (using C# MVVM). There are void methods that interact with various hardware components using their packages (e.g. National Instruments, OPC clients, etc.). There are also methods that take some input, do a GET or PUT to our API, and then return a response - these methods use HttpClient or something similar. These methods are not simple operators on the input, like Math.Sqrt(), and do not change states.

So, should such methods (and, in this case, the entire class) be static? There is the obvious benefit of having a static class with static methods: it is safe and fast because you don't have to create an object. Also, each of these static methods uses more static methods and classes for API and hardware interactions. The only downside that I see is that I would have to write unit tests with a paid framework, like TypeMock Isolator. Cost is not a problem if the answer is to mock them using something like TypeMock Isolator or some similar paid service, so if that is the consensus, that's fine. I just want to make a decision that will scale well and and leave little technical debt behind as we bring on new devs and as our project grows.

Let me know if I need to provide more information to make this more clear!

  • 3
    "interact with various hardware components", "There are also methods that take some input, do a GET or PUT to our API, and then return a response" -> these are the poster child examples of interacting with state – Caleth Jun 5 '18 at 15:50
  • 2
    To expand on @Caleth's comment: Remember that "state" does not just apply to the ones and zeroes in memory. If you call a function that moves a robot's arm, you've changed state --- in the real world --- along with all of the ramifications of doing so. – Greg Burghardt Jun 5 '18 at 16:01
  • 1
    "safe and fast because you don't have to create an object". In the network API case, you are certainly creating many objects inside the call. And, in any case, the time to create your object is small compared to network latency. – user949300 Jun 5 '18 at 16:08
  • What are you going to do when you have more than one of the same hardware component? Copy/paste all the methods? – immibis Jun 5 '18 at 23:56
15

So, should such methods (and, in this case, the entire class) be static?

No. Or at least, you shouldn't use them directly as statics.

If you're working with various hardware components, you're very very likely to want to mock them, use new ones, and pick which one you're using. That all shouts for having an interface (or other abstraction) so the rest of your code can promptly ignore that there's some hardware component at all. And statics in most languages play very poorly with interfaces (and other abstraction mechanisms).

  • 1
    Or at least, it is common to have collections of static methods that make the actual calls that talk to external stuff, but then to build a stateful API class on top of it, and expose that class as the way to get things done, and it's that class that gets mocked rather than the underlying interfaces, i.e. facade pattern. – whatsisname Jun 6 '18 at 1:27
  • Thanks for the reply! This seems to be the consensus, so I will adhere to this principle going forward. – adjordan Jun 6 '18 at 15:18
4

The conflicting pieces of advice you've read all make sense in a way, but they all make (different) assumptions or ambiguously phrase things. It's one of those "saying the same thing with different words" types of distinctions.

a method should only be static if it does not modify a state

Note the ambiguity of "modify a state". The following example violates this rule (literally), but it preserves the (figurative) spirit of the rule:

public static void SetUserNameToBob(User u)
{
    u.Name = "Bob";
}

This modifies the state of the User object, so it does in fact "modify a state".

However, this method does not rely on a particular internal state in order to decide it's own logical flow (e.g. u.Name = currentlySelectedDefaultName() would be a violation as the selected name is a selected state). And I think that's what is meant: no internal state is modified.

[a method should only be static if] its result depends only on the parameters provided to it

Look at the previous item, this one is pretty much saying the same thing. What it means is that this:

public static string currentlySelectedDefaultName; 
public static void SetUserNameToBob(User u)
{
    u.Name = currentlySelectedDefaultName;
}

should not be static, as the "current default" name is a state, and therefore it should not be a global variable/method.

Think of what happens if two separate threads are running: one of them wants to default to "Bob", the other wants to default to "Jim". They will end up fighting over the value, which is liable to create massive debugging issues and unexpected behavior.
If, however, every thread has its own DefaultNameSetter object, then they don't fight over the same resource.

However, the most highly voted answer in this post states that static methods should be used whenever possible.

This is sort of enforcing a rule by try/failing. Can we set this method to static?

  • Yes => Good! Keep it that way!
  • No => This proves that the code is relying on a non-static value somewhere and therefore should not be made static.

To indirectly quote Jeff Goldblum in Jurassic Park, you should not argue the need for doing something merely by proving that it can be done.

Again, the approach is not necessarily (or always) wrong, but it blindly assumes that methods are already written to be as state-agnostic as logically possible, which is simply not the case.

Even if you subscribe to this methodological approach, you can still only apply it when a project is no longer in development. If the project is still in development, current methods may be placeholders for the future implementation. It might be possible to make Foo() static today, but not tomorrow, if the state-dependent logic simply hasn't been implemented yet.

Many of the answers in this post say that one should do whatever is most logical.

Well, they're not wrong; but isn't this just a minor rephrase from saying "do the correct thing"? That's not really helpful advice, unless you already know when to use statics and when to avoid them. It's a catch 22.


So when should you use statics?

As you've noticed, not everyone agrees on the rule, or at least on how to phrase the rule. I'll add another attempt here, but be aware that this is effectively creating another standard:

enter image description here

Keep that in mind.

Statics are universal truths.

That is the purpose of a global namespace: things that are correct across the entire application layer.

There is a slippery slope argument here. Some examples:

var myConfigKey = ConfigurationManager.AppSettings["myConfigKey"];

This is a very clear cut example. Application configuration inherently implies that the configuration is global to the application, and therefore a statis method is warranted.

bool datesOverlap = DateHelper.HasOverlap(myDateA_Start, myDateA_End, myDateB_Start, myDateB_End);

This method is universally correct. It does not care which dates you're comparing. The method does not care about the meaning of the dates. Whether they're employment dates, contract dates, ... doesn't matter to the method's algorithm.

Note the semantical similarity between contextual and state driven. Both sort refer to a "non universal" state. This proves that contextual differences therefore are state-dependent and not suitable to be made static.

var newPersonObject = Person.Create();

This is a universal truth. The same creation process is used all over the application.

However, this line can get blurred:

var newManager = Person.CreateManager();
var newJanitor = Person.CreateJanitor();

From a technical perspective, nothing has changed. Manager (and janitors) are created the same way all over the application. However, this has subtly created a state (manager/janitor), which slowly but steadily detracts from how universal the truth really is.

Can it be done? From a technical perspective; yes.
Should it be done? That's a matter of whether you're arguing the pure principle, or take into account that compromises need to be made in order to not pointlessly strive for logical perfection. So I would say if it doesn't create a bigger problem than the problem intends to solve.

As the options expand (managers, janitors, accountants, salespeople, ...), the problem grows. For a sufficiently large problem, a factory pattern is desirable.

If you only have two options and you have no reason to suspect that the list of options will grow, you can argue that the static creation methods suffice. Some might disagree, and I see their point too. But I tend to be practical and not overly perfectionist in my approach.

3

However, the most highly voted answer in this post states that static methods should be used whenever possible...

Static methods that effect state to which they are directly coupled are a very bad idea. They lead to global state - "spooky action at distance" - issues, spaghetti code and the like. They are difficult to test, debug and maintain. My reading of the most upvoted answer is not encouraging any of this, so all is well.

So, should such methods (and, in this case, the entire class) be static?

That depends. Let's take an example of the "void methods that interact with various hardware components using their packages". Those methods can be static, but only if they are decoupled from those packages. One way to achieve that is to pass those packages in as parameters. Want to test the method without the hardware? Easy, pass in a dummy, mocked, package that records actions, rather than accessing hardware in via the parameter. The same applies to the HTTP methods; as long as the actual means of accessing the API is passed in as a parameter, as is well.

But, are you really achieving anything this way over creating an instance and injecting those packages, HTTP classes etc in at construction time? Probably not. Sure, they are static methods, so you don't need an instance. But you have the added complexity of needing to expose all those packages etc throughout the code in order to pass them in as parameters. It's often far easier to create a single instance and pass that around instead.

  • You're saying to pass around a single instance of whatever "UtilityMethods.cs" class I make? I'm currently using dependency injection with SimpleIoC, which I think is the same idea. – adjordan Jun 6 '18 at 15:21
  • 2
    @adjordan, indeed. Whether you use pure (poor man's) DI or an IoC framework to assist with injection is up to you. The pinciple remains the same: it's easier to only pass around an instance of IUtilityMethods than to pass around multiple instances that need to be passed to static functions. – David Arno Jun 6 '18 at 15:29
1

What I recommend and what I am using within my team is to use static methods/classes if there is a real need for it. It should not be the first option to use static methods/classes. It has to be a very good reason for that. Many developers see the static option is faster and hence cheaper. But that is maybe in the beginning. The cost is to build a more complicated Unit Tests and lacking the ability to use contracts (interfaces) with multiple implementations. For the methods/classes that interact with hardware. These should not be static. First and for most, there is no reason. Second, it is required to mock these classes in unit tests for a simpler testing code. Third, It is possible to replace the implementation in the future.

0

Static functions are more challenging when you want to allow multiple access in parallel.

For example, it is perfectly reasonable, and likely a good idea, to allow multiple HTTP calls to execute at the same time. With an instance object, you can easily maintain different state for those calls - the headers, result codes, buffers, etc...

With a single static method, well, you may end up creating some internal instance to hold that state. Or you synchronize everything (I'm using Java terminology, YMMV), which is error-prone. Or use a queue. In any case, without some fancy footwork, you lose the ability to run in parallel. If one HTTP call bogs down due to a network glitch, all the others have to wait.

Same for your hardware - in some cases it might make sense to allow access in parallel.

For these reasons, I disagree with that 8 year old answer you cited.

  • 4
    A static function should not contain state at all. – Pieter B Jun 6 '18 at 8:17
  • 1
    @Pieter B Of course (other than perhaps a counter). In the HTTP example, it would have to create a MyHTTPHelper object per call to hold the state. At which point it's probably simpler to have the original caller create an object, not use a static method at all. – user949300 Jun 7 '18 at 21:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.