patterncsharpModerate
Single instancing class
Viewed 0 times
singleclassinstancing
Problem
I have created the following class that is used to provide single instancing functionality to an application. It is called like:
The idea of the class is to prevent the program from being ran twice and it does this by using the applications Guid, as it is a C# program these should always be present. It also allows for extra functionality so a user can override the Guid, this is useful if the same program has different modes it can be ran in so they can both run at the same time.
Lastly it provides the functionality to show the running app, either from the Guid, or from strings contained in the applications main window title.
Here's the class:
```
using System;
using System.Diagnostics;
using System.Drawing;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Security.AccessControl;
using System.Security.Principal;
using System.Threading;
using System.Windows.Forms;
namespace Helpers
{
using static User32;
///
/// A class that provides functionality for single instancing an application.
///
public static class SingleInstance
{
#region Constants
private const uint WINDOW_FLASH_COUNT = 6;
private const uint WINDOW_FLASH_RATE = 70;
#endregion
#region Fields
private static string __appGuid;
private static Mutex _mutex;
#endregion
#region PublicProperties
///
/// If true the application checks if it is running only against the current user, else it checks against all users.
///
public static bool ThisUserOnly { get; set; } = true;
#endregion
#region PrivateProperties
private static string AppGuid
{
get
{
return __appGuid;
}
set
{
__appGuid = value
if (Helpers.SingleInstance.IsAlreadyRunning())
{
Helpers.SingleInstance.ShowRunningApp();
return;
}The idea of the class is to prevent the program from being ran twice and it does this by using the applications Guid, as it is a C# program these should always be present. It also allows for extra functionality so a user can override the Guid, this is useful if the same program has different modes it can be ran in so they can both run at the same time.
Lastly it provides the functionality to show the running app, either from the Guid, or from strings contained in the applications main window title.
Here's the class:
```
using System;
using System.Diagnostics;
using System.Drawing;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Security.AccessControl;
using System.Security.Principal;
using System.Threading;
using System.Windows.Forms;
namespace Helpers
{
using static User32;
///
/// A class that provides functionality for single instancing an application.
///
public static class SingleInstance
{
#region Constants
private const uint WINDOW_FLASH_COUNT = 6;
private const uint WINDOW_FLASH_RATE = 70;
#endregion
#region Fields
private static string __appGuid;
private static Mutex _mutex;
#endregion
#region PublicProperties
///
/// If true the application checks if it is running only against the current user, else it checks against all users.
///
public static bool ThisUserOnly { get; set; } = true;
#endregion
#region PrivateProperties
private static string AppGuid
{
get
{
return __appGuid;
}
set
{
__appGuid = value
Solution
I've read through this several times, and I see a couple things that bother me.
First off, this class name just doesn't make sense. I hate to say it, but it really doesn't. When I see
So now that we've named it
Technically
Once we clear that up, there's an obvious enhancement that could be made to the API:
This means in cases where we don't need to
Looking more into the API I really don't like the idea of having the
Instead, you could supply a
If you make these changes then this:
Becomes:
Just as well,
If you don't want to switch those properties out for parameters, then you should make this an instance class. I see absolutely zero work inside it that could not be done in an instantiated class. Then you can define a constructor
First off, this class name just doesn't make sense. I hate to say it, but it really doesn't. When I see
class SingleInstance I immediately think Singleton which is not what this is, this is an InstanceManager.So now that we've named it
InstanceManager (or something similar, that was the first name that came to mind that described what it did) we need to look at the API a bit, because some of that rubs me the wrong way as well.if (Helpers.InstanceManager.IsAlreadyRunning())
{
Helpers.InstanceManager.ShowRunningApp();
return;
}Technically
IsAlreadyRunning is not checking for this app, but another app of the same type. So it should be IsInstanceRunning(), then the verbiage makes a little more sense.Once we clear that up, there's an obvious enhancement that could be made to the API:
public static void ShowIfInstanceRunning()
{
if (IsInstanceRunning())
{
ShowRunningApp();
}
}This means in cases where we don't need to
return or anything if the other is running we can simply do:Helpers.InstanceManager.ShowIfInstanceRunning()Looking more into the API I really don't like the idea of having the
ThisUserOnly (which should be CurrentUserOnly) be a property, as well as the AppGuid and requiring an OverrideGuid method. That's just ugly.Instead, you could supply a
Guid? parameter to various methods, if guid.HasValue == false then use the AppGuid, if guid.HasValue == true then use guid.Value. This eliminates the need for OverrideGuid, which is a very ugly API.If you make these changes then this:
public static void OverrideAppGuid(string text)
{
using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create())
{
OverrideAppGuid(new Guid(md5.ComputeHash(System.Text.Encoding.Default.GetBytes(text))));
}
}Becomes:
public static Guid GenerateHashedGuid(string text)
{
using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create())
{
return new Guid(md5.ComputeHash(System.Text.Encoding.Default.GetBytes(text)));
}
}Just as well,
ThisUserOnly should be a parameter bool currentUserOnly = true on the appropriate methods.If you don't want to switch those properties out for parameters, then you should make this an instance class. I see absolutely zero work inside it that could not be done in an instantiated class. Then you can define a constructor
InstanceManager(Guid) that you can attach directly to the appropriate application so that if a user wants to check for multiple applications running, it's as simple as:var thisInstanceManager = new InstanceManager();
var xInstanceManager = new InstanceManager(xGuid);Code Snippets
if (Helpers.InstanceManager.IsAlreadyRunning())
{
Helpers.InstanceManager.ShowRunningApp();
return;
}public static void ShowIfInstanceRunning()
{
if (IsInstanceRunning())
{
ShowRunningApp();
}
}Helpers.InstanceManager.ShowIfInstanceRunning()public static void OverrideAppGuid(string text)
{
using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create())
{
OverrideAppGuid(new Guid(md5.ComputeHash(System.Text.Encoding.Default.GetBytes(text))));
}
}public static Guid GenerateHashedGuid(string text)
{
using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create())
{
return new Guid(md5.ComputeHash(System.Text.Encoding.Default.GetBytes(text)));
}
}Context
StackExchange Code Review Q#154816, answer score: 12
Revisions (0)
No revisions yet.