Singleton class, use another class?

[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

 

[Guest]

Sure. Why wouldn't it be ok?

 

[Brian Gideon]

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

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.

 

[Marc Gravell]

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

 

[Guest]

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.

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

 

[Guest]

Hi,

Thanks for your reply.

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:

Would the following be enough to seal the class correctly -

namespace MyApp
{
sealed class SingletonConnection
{

}
}

Cheers,
Dav

 

[Brian Gideon]

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

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;}}

 

[Marc Gravell]

There is a full discussion of various approaches (pros, cons, etc)
here:

http://www.pobox.com/~skeet/csharp/singleton.html

Marc

 

[Marc Gravell]

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

 

[Guest]

Thanks for this link. Its very useful for me.

David

 

[Marc Gravell]

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