I have a math helper class where every single function is static, i.e., params fed in as arguments, value returned. Should I declare the entire class as static? Would adding the static modifier to the class make a difference in performance?
Also, I am not sure what this guideline means in: "do not treat static classes as a miscellaneous bucket." - I have a few classes that are just a bunch of miscellaneous static functions...
It's perfectly fine to make classes like that static, in fact if you look at System.Math you'll see it's static as well:
public static class Math
What the guideline is trying to say is you should not put every static method you have to one static class which would do everything and play a role of a bucket for static methods. Instead, if it's appropriate, create smaller util classes with methods related to the same functionality, like it's done with System.Math and couple more within BCL as well.
Should I declare the entire class as static?
Yes. Adding static to a class says that it contains only static members and that you can't ever instantiate it. Without it, users of your class might get confused and try to create an instance or variable of your class. With static, that's not possible.
It seems like this is exactly your case.
Would adding the static modifier to the class make a difference in performance?
No, call to a static method will always have the same performance characteristics, it doesn't matter whether the containing class is static or not. Actually, the whole concept of static classes doesn't exist at the CIL level, they're just sealed abstract classes (a combination that wouldn't compile in C#).
But even if there was a difference, it would be tiny. Don't optimize prematurely, especially when it comes to micro-optimizations.
Helper classes are normally static classes, so that you don't need to instantiate them. There is no great cost in instantiating a managed .NET object (especially helper classes), it is just a matter of convenience.
It is extremely tempting to just put together a static class with minimal helper methods and get the job done. They have their place in code, and can be used especially when there is deterministic input/output. e.g. ComputeHash of a string, Find Average of numbers etc.
But the one reason, Static classes are discouraged is because they normally interfere with unit testing and present all sorts of problems. (Fakes, Moles, Private Accessors etc.)
An interfaced based approach for even helper classes, helps with the unit testing of the overall code. This is especially true for big projects which involve workflows such that the static helper methods are only a part of the workflow.
e.g. Suppose you need to check if the current year is a leap year. It is tempting to write a quick static method.
public static class DateHelper
{
public static bool IsLeapYear()
{
var currentDate = DateTime.UtcNow;
// check if currentDate's year is a leap year using some unicorn logic
return true; // or false
}
}
and if this method is used in your code somewhere like:
public class Birthday
{
public int GetLeapYearDaysData()
{
// some self-logic..
// now call our static method
var isLeapYear = DateHelper.IsLeapYear();
// based on this value, you might return 100 or 200.
if (isLeapYear)
{
return 100;
}
return 200;
}
}
Now, if you go and try to unit test this method public int GetLeapYearDaysData(), you might end up in trouble since the return value is indeterminate.. i.e. depends on the current year and it is not recommended to have unit tests behaving unpredictably/deteriorate as time progresses.
// this unit test is flaky
[Test]
public void TestGetLeapYearDaysData()
{
var expected = 100;
// we don't know if this method will return 100 or 200.
var actual = new Birthday().GetLeapYearDaysData();
Assert.AreEqual(expected, actual);
}
The above problem happens because we cannot control/mock the method IsLeapYear() in the above code. so we're at its mercy.
Now imagine the following design:
public interface IDateHelper
{
bool IsLeapYear();
}
public class DateHelper : IDateHelper
{
public bool IsLeapYear()
{
var currentDate = DateTime.UtcNow;
// check if currentDate's year is a leap year using some unicorn logic
return true; // or false
}
}
Now our birthday class can be injected with a helper:
public class Birthday
{
private IDateHelper _dateHelper;
// any caller can inject their own version of dateHelper.
public Birthday(IDateHelper dateHelper)
{
this._dateHelper = dateHelper;
}
public int GetLeapYearDaysData()
{
// some self-logic..
// now call our injected helper's method.
var isLeapYear = this._dateHelper.IsLeapYear();
// based on this value, you might return 100 or 200.
if (isLeapYear)
{
return 100;
}
return 200;
}
}
// now see how are unit tests can be more robust and reliable
// this unit test is more robust
[Test]
public void TestGetLeapYearDaysData()
{
var expected = 100;
// use any mocking framework or stubbed class
// to reliably tell the unit test that 100 needs to be returned.
var mockDateHelper = new Mock<IDateHelper>();
// make the mock helper return true for leap year check.
// we're no longer at the mercy of current date time.
mockDateHelper.Setup(m=>m.IsLeapYear()).Returns(true);
// inject this mock DateHelper in our BirthDay class
// we know for sure the value that'll be returned.
var actual = new Birthday(mockDateHelper).GetLeapYearDaysData();
Assert.AreEqual(expected, actual);
}
As you can see, the moment the helper methods were Interface based, they were easily testable. Over the course of a big project, many such smaller static methods ultimately result in bottlenecks in testing key functional flows.
So it pays to be aware of this pitfall in advance and make the additional investment upfront. Basically identify what classes/methods need to be static and what shouldn't be.
It all starts from when should I have a static method, and that is when you don't have any dependency on instance variables.
Now that said if none of you methods are depending on instance variable, you can make your class static.
Static class serve several benefits, and many more.
