0

I am having a singleton class in my code.
In my main function, I created an object of that class.
Then, I tried to create the clone for that object.
And, it gave me "StackOverflowException".

My code looks like:

namespace SingletonApplication
{
    class A : ICloneable
    {
        private static readonly A A1 = new A();

        public A A2 { get { return A1; } }
        public object Clone()
        {
            var obj = ((ICloneable)A1).Clone();
            return obj;
        }
    }


    class Program
    {
        static void Main(string[] args)
        {
            A obj1 = new A();
            A obj2 = (A)(obj1.Clone());

            Console.WriteLine(object.ReferenceEquals(obj1.A2, obj2.A2));

            Console.ReadKey();
        }

    }
}

Error: enter image description here

Ajay
  • 643
  • 2
  • 10
  • 27

4 Answers4

2

Singleton, by definition, is meant to be a class with only one instance across entire application.

The StackOverflowException you get is caused by Clone menthod which keeps calling itself.

Alex
  • 1,433
  • 9
  • 18
1

The requirments are contradictory ones:

  • Singleton can have at most one instance by its own definition
  • Clone() method is supposed to produce a clone, a new (== second) instance

Probably a better solution is to return a fake clone (i.e. itself)

   // sealed: do not inherit from me (I'm a Singleton) and create a second instance
   sealed class A : ICloneable
   {
       private static readonly A A1 = new A();

       //private constructor: do not create instances (I'm a Sinleton)
       private A() {}

       public A A2 { get { return A1; } }

       // We can't create a new clone but can return an existing instance
       public object Clone()
       {
           // Cloned Singleton? Let it be itself
           return this;
       }
   }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
0

Instead of recursively calling Clone on the same object over and over again, you should actually do something to clone that object.

Usually you create a new instance and copy all relevant properties over. Like this:

A newA = new A();
newA.X = this.X;

return newA;

However, there is nothing to copy in your code. The readonly field will stay as it is. I wonder why you need a copy of a singleton since that defeats the purpose of the design pattern. And by the way, you implementation of singleton is quite unique. I advise to read up on singletons and follow some samples there.

Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
  • Followed one of the stackoverflow answer [http://stackoverflow.com/a/12367609 ] ..... As per design pattern, it is never intended – Ajay Oct 07 '16 at 06:58
0

The error is caused because You let Clone call itself. resulting in an endless loop And This is not a singleton. A singleton should look like this

class A 
    {
        private static A instance;

        private A() { }

        public static A Instance
        {
            get
            {
                if (instance == null)
                {
                    instance = new A();
                }
                return instance;
            }
        }
    }
Pepernoot
  • 3,409
  • 3
  • 21
  • 46