Search code examples
c#class-design

Is it bad practice to only create static methods that take instances of that type


Take this class for example:

public class Account
{
    public string Code { get; set; }
    public string Description { get; set; }
    public DateTime CreatedOn { get; private set; }  

    public void Create()
    {
        // Invoke with new Account({...}).Create();
    }

    //Or

    public static void Create(Account account)
    {
        // Invoke with Account.Create(new Account({...}));
    }
}

Both Create() methods will both do the same thing, but as you can see they are invoked differently. Is one better practice over the other? Is there a term for writing code like this?


Solution

  • In general, I don't know that this is either good or bad practice. However, I would lean towards 'bad practice' in the example you have given (and I can't off hand think of any useful reasons to pass an instance of a type to a static method declared on that type).

    Certainly in your example, you are (IMO) creating an unclear API. I'm not sure whether;

    • Account.Create is meant to issue data storage requests for an account (functionality that certainly belongs elsewhere in your object model)
    • is some sort of 'convenience' method for creating an instance of Account using some default set of parameters (which would arguably be better in a constructor of Account)
    • is meant as a copy constructor (in which case, it should be a constructor)
    • is a cheap implementation of an AccountFactory (in which case you should make a factory!)

    Another consideration is testing - static methods can introduce difficulties when unit testing anything that calls them, as they may not be easy to mock or stub.

    Apart from those, there is nothing inherently 'wrong' about this, technically. It affects your design in other ways though (e.g; no access to instance members as it is not an instance).

    So, I think this approach can lead to some confusion, however, it might also be entirely appropriate for your situation!