0

I have a class below that I implemented as a Singleton, and added few methods to use with and without parameters, leaving off the details:

public class LogManager
{
    public static LogManager Instance
    {
        get
        {
            return instance;
        }
    }

    public LogManager Create(Guid productId)
    {
        _productId = productId;
        return Instance;
    }

    public LogManager Create()
    {
        return Instance;
    }

    public void WriteStateChangeEvent()
    {
    }
}

Is there any way to restrict calling Instance directly so it can be called like this LogManager.Instance.Create(newProductId).WriteStateChangeEvent() or LogManager.Instance.Create().WriteStateChangeEvent() but never like this LogManager.Instance.WriteStateChangeEvent()

This is not a general OOP Singleton pattern question but a question about specific implementation

Victor
  • 985
  • 2
  • 28
  • 53
  • Unlike forum sites, we don't use "Thanks", or "Any help appreciated", or signatures on [so]. See "[Should 'Hi', 'thanks,' taglines, and salutations be removed from posts?](http://meta.stackexchange.com/questions/2950/should-hi-thanks-taglines-and-salutations-be-removed-from-posts). – John Saunders Jun 20 '14 at 19:02

1 Answers1

0

You shouldn't actually have a singleton here, because you don't conceptually have a single object.

You should simply expose a constructor for the type publicly, possibly with a factory Create method if you really need it, and then have WriteStateChangeEvent called on each of those separately initialized (and by the looks of it, immutable) instances.

Having a single instance of the class represent multiple conceptual instances of the class by simply "reusing" it over and over is confusing to users of the class and very error prone.

If there is any state that is really conceptually independent of all of the instances, then that can be represented in static fields.


The alternative approach, assuming there is some other important reason to have a singleton, would be to remove the Create method entirely and simply pass productId as a parameter to WriteStateChangeEvent, so that you are no longer storing state that is tied to a conceptual instance.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Thanks and I agree, but that's a requirement - to implement this class as a Singleton. – Victor Jun 20 '14 at 19:05
  • @Victor Requirement for what reason? The class is conceptually not a singleton. It has state specific to individual conceptual instances, you're just trying to re-use a single instance to do it, and that's (understandably) causing problem. – Servy Jun 20 '14 at 19:07
  • Just a requirement I have to work with. Again, I agree with your statement but cannot change it. – Victor Jun 20 '14 at 19:10
  • @Victor Where does the requirement come from? *Why* do you need to use a singleton? The actual underlying constraint(s) affect how you should actually go about trying to solve the problem. – Servy Jun 20 '14 at 19:20
  • They come from product owners that are technical enough to know what the Singleton is but not enough to understand Object states – Victor Jun 20 '14 at 19:25
  • @Victor So your answer is an incompetent architect? In that case, explain to them (politely) why a singleton is the wrong tool for the job (or just ignore them and use the right tool anyway, if you're in a position to do that). – Servy Jun 20 '14 at 19:27