1

Is it a good practice to pass instances of classes which are created only once at application start to other classes over a static container class which only has getter and setter to get the instance or would that be a use case for singletons?

In my case the class is a representation of the currently logged in user.

Something like this for example:

class ClassA
{}

class ClassB
{
    public void UseClassA()
    {
        ClassA classA = Container.ClassA;
    }
}

class ApplicationStart
{
    public void Main()
    {
        Container.ClassA = new ClassA();
    }
}

static class Container
{
    private static ClassA classA;
    public static ClassA ClassA
    {
        get { return classA; }
        set { classA = value; }
    }
}
rianjs
  • 7,317
  • 5
  • 21
  • 36
Yannik H
  • 46
  • 5
  • 1
    looks like you are trying to implement singleton pattern – Rahul Jul 19 '17 at 11:13
  • If you have classes which must have only instance then you should use Singleton pattern – Samvel Petrosov Jul 19 '17 at 11:13
  • 5
    I recommend you to use IoC frameworks for managing your objects lifetime. Whilst it is a bit comples at start to understand how it works - it is very poverful feature which free your code from static cycle dependencies. – eocron Jul 19 '17 at 11:14
  • 1
    Just a note https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons – vasek Jul 19 '17 at 11:15
  • 1
    Static members are almost always a bad practice. If we talk about currently authenticated user, then it depends on what application we are talking about. I would say that "currently authenticated user" can be a static member / singleton member for WinForms or WPF environments, but it should be read-only (it should be set by an authentication mechanism, not manually). – Yeldar Kurmangaliyev Jul 19 '17 at 11:15

2 Answers2

1

Is it a good practice to pass instances of classes which are created only once at application start to other classes

Yes! That's called dependency injection (DI), because you inject each object's dependencies as constructor parameters. The application entry point is called the "composition root" in DI parlance:

A Composition Root is a (preferably) unique location in an application where modules are composed together.

Passing objects to other objects via the constructor is called the Constructor Injection Pattern.

over a static container class which only has getter and setter to get the instance or would that be a use case for singletons?

Using singletons is almost certainly worse.

Here's Mark's commentary on a similar question:

You are right that if you use the container as a Service Locator, it's more or less a glorified static factory. For lots of reasons I consider this an anti-pattern.

One of the wonderful benefits of Constructor Injection is that it makes violations of the Single Responsibility Principle glaringly obvious.

When that happens, it's time to refactor to Facade Services. In short, create a new, more coarse-grained interface that hides the interaction between some or all of the fine-grained dependencies you currently require.

Creating a "real" singleton class is almost certainly worse than instantiating your object, and handing it to the other objects that need it as a constructor parameter.

In your case, I would do something like this:

static void Main()
{
    // This is your composition root
    var numberProvider = new NumberProvider();
    var b = new B(numberProvider);
    b.DoSomething();
}

class NumberProvider
{
    private int _number = 0;

    // In the real world, maybe this gets a value from a web service
    // and implements an INumberProvider interface so you can mock it
    // for unit testing
    public int GetNumber() => _number++;
}

class B
{
    private readonly NumberProvider _numberProvider;

    public B(NumberProvider numberProvider)
    {
        _numberProvider = numberProvider;
    }

    public void DoSomething()
    {
        Console.WriteLine(_numberProvider.GetNumber());
    }
}

Avoid the public setters. And chances are, you don't need everything to be public.

rianjs
  • 7,317
  • 5
  • 21
  • 36
0

So, you're asking if it's better to have separate singletons, like this:

public class Singleton1
{
    public static Singleton1 Instance { get; set; }
}

public class Singleton2
{
    public static Singleton2 Instance { get; set; }
}

Or a single Container that takes all them in one place:

public static class Container
{
    public static Singleton1 FirstSingleton { get; set; }
    public static Singleton2 SecondSingleton { get; set; }
}

Since they are tecnically the same, this is an opinion-based dilemma. It depends about how do you feel about one or the other.

A more interesting question could be: should I either use a static Container, or a more advanced (but more complicated) tecnique like Dependency Injection with a Dependency Container? But again, it depends on your needs. If you need just a couple of simple Singletons, a simple static Container is just fine (remember the golden KISS principle: Keep It Simple, Stupid!). Unless you're using it to store special objects, like an authenticated user: in that case, it should be managed by the Authentication classes and policies, as @Yeldar pointed out.

Massimiliano Kraus
  • 3,214
  • 5
  • 20
  • 40