0

I have a winform constructor with parameter:

    public SnippingTool(Bitmap screenShot)
        {
            InitializeComponent();
            this.BackgroundImage = screenShot;
            ...
        }

I need only one instance of my winform while app is running, so I decided to use singleton pattern.

I found this construction (it's not suitable because of parameter in my class constructor):

    private static SnippingTool instance;

    public static SnippingTool GetInstance
    {
        get
        {
            if (instance == null)
            {
                instance = new SnippingTool();
            }
            return instance;
        }
    }

How can I pass parameter through singleton?

  • 1
    Change the property call to a method call and make the method callable with a parameter? – Ian H. Jan 09 '18 at 08:35
  • 1
    I'd make a Bitmap property in the singleton and just set it once, throw an error if it hasn't been set. – EpicKip Jan 09 '18 at 08:36
  • 1
    Why you have a property `IsDisposed`? – Tim Schmelter Jan 09 '18 at 08:43
  • 1
    Why not create it once and when you need it make sure to pass it along as a parameter/similar? No need to implement singleton IMO. (This is usually called dependency injection) – default Jan 09 '18 at 08:43
  • 2
    The answer to this depends on where you will get `screenShot` from - is it available only at the time that you call `GetInstance`? Or is it available before then? – Matthew Watson Jan 09 '18 at 08:55
  • 2
    Instead of singleton of the type instance itself, consider to store it somewhere else, e.g. as `App.MainForm` property. Now you can construct it explicitly somewhere during application startup. Note: keep instance `null` and allow software to crash (with logging of course) to catch cases when something is trying to access instance *before* it is initialized. – Sinatr Jan 09 '18 at 08:55
  • 1
    Thus, answers present here is very simple, I tend to use inversion of control even for winforms. Their lifetime can very frustrating. Also, disposing winforms is a pain, so I recommend you to look at solution present here: https://stackoverflow.com/questions/38417654/winforms-how-to-register-forms-with-ioc-container – eocron Jan 09 '18 at 09:08
  • @TimSchmelter thanks, there is no need for `IsDisposed` –  Jan 09 '18 at 10:50
  • @MatthewWatson thanks, I've complemented my question –  Jan 09 '18 at 10:52

5 Answers5

1

If the Bitmap is different every time, and you definitely want only one SnippingTool, then maybe the Bitmap should not even have an instance variable of the tool, and then certainly the Bitmap has no place in the Constructor.

Instead make the Bitmap a parameter of the main 'operating' method(s) that you are calling on the SnippingTool instance.

Or if you need the SnippingTool to have a Bitmap as 'state' (because it is a Form, and it needs to show & edit the image), then create methods such as SetImage(Bitmap b), ClearImage(), etc.

Peter B
  • 22,460
  • 5
  • 32
  • 69
0
private static SnippingTool instance;

public static SnippingTool GetInstance(Bitmap screenShot)
{
        if (instance == null || instance.IsDisposed)
        {
           instance = new SnippingTool(); 
        }
        instance.SetBitmap(screenShot); // if you need otherwise pass it to constructor
        return instance;
}
Jester
  • 3,069
  • 5
  • 30
  • 44
  • I guess, you need a semicolon after `instance.SetBitmap(screenShot)` –  Jan 09 '18 at 10:45
0

You can also create a method instead of property.

private static SnippingTool _instance;

public static SnippingTool GetInstance(Bitmap screen)
{
    get
    {
        if (_instance == null || _instance.IsDisposed)
        {
            _instance = new SnippingTool(screen);
        }

        return _instance;
    }
}
0

Try this

 private static SnippingTool instance;

 private SnippingTool (var par){
   ....
 }

 public SnippingTool createSingleton(var par){
   if instance != null 
               return instance;
   else 
      return new SnippingTool(par);
 }
S-Wing
  • 485
  • 6
  • 25
0

If this SnippingTool can be used only with one BitMap instance there is no reason to always pass one to the singleton property/method. That would at least be confusing because the caller would think that he would get a tool for this bitmap. The method should throw an exception if you pass a different instance. But does this make sense at all, not much in my opinion.

So i would go this way, implement a SetScreenshot or SetInstance method, f.e.:

public class SnippingTool
{
    private SnippingTool(Bitmap screenShot)
    {
        this.ScreenShot = screenShot;
    }

    private static SnippingTool instance;

    public static void SetScreenshot(Bitmap screenShot)
    {
        if (instance == null || instance.IsDisposed)
        {
            instance = new SnippingTool(screenShot);
        }
        else
        {
            // now your logic, is it allowed then overwrite the screenShot, otherwise:
            throw new ArgumentException("It's not allowed to change the screenShot for this SnippingTool", nameof(screenShot));
        }
    }

    public static SnippingTool GetInstance
    {
        get
        {
            if (instance == null)
                throw new Exception("The SnippingTool needs a screenShot first, use first " + nameof(SetScreenshot));
            return instance;
        }
    }

    public Bitmap ScreenShot { get; }

    // ...
}
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939