Singleton class, use another class?

  • Thread starter Thread starter Guest
  • Start date Start date
G

Guest

Hi,

I have created a Singleton class to provide some database functionality in
my mobile application. I have a public class called Utility which performs
various operations on data. Is it ok to use the utility class in my Singleton
class? Here is some code to hopefully make it clearer what i want to do -

namespace MyApp
{
class SingletonConnection
{
private volatile static SingletonConnection instance;

private Utility utility;

private SingletonConnection()
{
}

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

public void DoStuff()
{
utility = new Utility();

utility.DoStuff();
}
}

class Utility
{
public Utility()
{
}

public void DoStuff()
{

}
}
}

Thanks for any tips,
Dav
 
Hi,

I have created a Singleton class to provide some database functionality in
my mobile application. I have a public class called Utility which performs
various operations on data. Is it ok to use the utility class in my Singleton
class? Here is some code to hopefully make it clearer what i want to do -

namespace MyApp
{
class SingletonConnection
{
private volatile static SingletonConnection instance;

private Utility utility;

private SingletonConnection()
{
}

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

public void DoStuff()
{
utility = new Utility();

utility.DoStuff();
}
}

class Utility
{
public Utility()
{
}

public void DoStuff()
{

}
}

}

Thanks for any tips,
Dav
Click to expand...

Yes, it is okay to use the Utility class (or any other class for that
matter) if it helps your implementation of the singleton.

As written your singleton is not thread-safe. I would remove the
volatile modifier and wrap the contents of the GetInstance method with
a lock construct. Better yet, create a new SingletonConnection
instance inline with the declaration and have the GetInstance method
just return the reference. Or if you don't require any fancy
initialization or polymorphism then just create a static class.
 
Since DoStuff() creates a new instance each time, why not just hold
this as a local variable? Unless I missed something in the usage, this
could also be implemented just as a static method. A lot here depends
on whether utility is itself thread-safe...

Note that your SingletonConnection is not guaranteed to be singleton.
First, you might want to seal it, and second there is a race-condition
on the first access; if two threads get past the null check you could
get 2. Unless there is a huge cost associated with initializing a
SingletonConnection (which the ctor doesn't indicate), then perhaps
just use:

private static readonly SingletonConnection instance = new
SingletonConnection();
public static SingletonConnection Instance {get {return instance;}}

Of course, unless the singleton is implementing a specific base /
interface, you might be able to get away with just a static class with
static methods (etc).


Did I miss the point?

Marc
 
Brian Gideon said:
On Nov 19, 7:49 am, davebythesea

snip

Yes, it is okay to use the Utility class (or any other class for that
matter) if it helps your implementation of the singleton.

As written your singleton is not thread-safe. I would remove the
volatile modifier and wrap the contents of the GetInstance method with
a lock construct. Better yet, create a new SingletonConnection
instance inline with the declaration and have the GetInstance method
just return the reference. Or if you don't require any fancy
initialization or polymorphism then just create a static class.
Click to expand...

Thanks for the tips. I think i saw some code which used the lock construct
so I will probably hunt for it again and add it in there.

Cheers
 
Hi,

Thanks for your reply.

Marc Gravell said:
Note that your SingletonConnection is not guaranteed to be singleton.
First, you might want to seal it, and second there is a race-condition
on the first access; if two threads get past the null check you could
get 2. Unless there is a huge cost associated with initializing a
SingletonConnection (which the ctor doesn't indicate), then perhaps
just use:
Click to expand...

Would the following be enough to seal the class correctly -

namespace MyApp
{
sealed class SingletonConnection
{

}
}

Cheers,
Dav
 
Thanks for the tips. I think i saw some code which used the lock construct
so I will probably hunt for it again and add it in there.

Cheers
Click to expand...

Unless you have a specific reason for doing manual synchronization I
would stick with what Marc posted.

private static readonly SingletonConnection instance = new
SingletonConnection();
public static SingletonConnection Instance {get {return instance;}}
 
yep; actually, this only becomes necessary if there is a non-private
constructor (since you can't inherit from something with only private
constructors), but it is a good idea anyway

Marc
 
For the link: you're welcome - there is a lot of good content on that
site on a range of C# topics - definitely recommended reading

For the content: [pings the "thanks" in Jon's direction]

Marc
 
Back
Top