0

We have a class "DataAccessServiceConnector", in which we have few methods to communicate with Data Access Service.

public class DataAccessServiceConnector: IDataAccessServiceConnector
{
     public async Task<HttpResponseMessage> GetDataAccessServiceResponse()
     {
        //Some code
        return GetDataFromDataAccessService();
     }        
}

We have an Interface:

public interface IDataAccessServiceConnector
{
    Task<HttpResponseMessage> GetDataAccessServiceResponse();
}

And having a different class, that is holding the instance of "DataAccessServiceConnector" class in the as static object.

public class ClassA
{
  public static IDataAccessServiceConnector DataAccessConnector;
  //Constructor of the Class
  ClassA()
  {
     DataAccessConnector = DataAccessConnector ?? new DataAccessServiceConnector();
  }
}

Is it bad practice to hold the class instance (i.e. DataAccessServiceConnector) in a static object(i.e. DataAccessConnector)?

Ankit Jain
  • 315
  • 3
  • 18
  • 1
    I don't see the harm unless your code was designed without thread safety in mind. You are basically describing a singleton, which is well-used in programming, except you have a class that could also *not* be a singleton. – Ron Beyer May 10 '18 at 17:37
  • 1
    This is a kind of implementation pattern of singleton. If you want to make it `singleton`, then I don't think it is a bad practice as this is the way of implementation. – Ritwick Dey May 10 '18 at 17:39
  • @RonBeyer: Is there any Thread safety issue with this implementation? – Ankit Jain May 10 '18 at 17:50
  • @AnkitJain The only thing I see is that `ClassA`'s constructor needs to be marked `static`, and the entire class should be marked `static` as well. Without seeing the actual implementation of `GetDataAccessServiceResponse`, and it's containing class, it is hard to say if it is thread safe or not. – Ron Beyer May 10 '18 at 17:54

2 Answers2

-1

I think, the answer to your question is opinion-based in context of StackOverflow.

This is a classic "Is Singleton Pattern Bad?" question:


There are numerous issues with Singletons in particular and with shared-write-access memory in general.

  • As others mentioned above, sharing resources requires extreme caution;
  • Similarly, [unit] testing of such class is not easy;
  • The (inter)dependencies between classes are less discoverable;
  • The code is harder to follow;
  • The bugs are harder to fix;
  • Code refactoring becomes harder in certain cases;
  • ...this list goes and goes on.

I personally decided to use Singletons as a last resort solution only. Even then, I would rely on a dependency injection framework's singleton registration instead of a static field. (E.g. SimpleInjector's singleton registration.)

But before registering something as a Singleton, please reconsider using regular objects first.

Igor Soloydenko
  • 11,067
  • 11
  • 47
  • 90
-1

Option 1.

If there is already a top level object such as 'Program' or 'Application' that everyone already has access to, you can make the instance object a new field or property on it.

Option 2.

Provide static access to the instance object, from the instance object, as a static method/field/property.

Or, as you have described, provide static access to the instance object from some other arbitrary type, if doing so is convenient for your design. The class heirarchy as you have described it sounds perfectly normal to me, actually.

James
  • 1,973
  • 1
  • 18
  • 32