principlecsharpMinor
Design Pattern Question/Violating OCP
Viewed 0 times
violatingocpdesignquestionpattern
Problem
Is there a design pattern I can use for a situation like below. This is an example I made up to try and explain the situation I'm dealing with currently. I keep having to add new DeviceScript classes to my DeviceAnalyzer as situation come up and adding logic about when to return the new type in the 'GetScriptForDevice'.
This example I have given is pretty simple but the business logic I'm having to implement for my real 'GetScriptForDevice' isn't always this easy. Any suggestions on a pattern or an example of a better way to structure this code to follow a better pattern would be greatly appreciated. I'm violating OCP every time I need to add a new DeviceScript and I'm trying to find a way around this because my class is getting quite long and fragile.
Thanks.
This example I have given is pretty simple but the business logic I'm having to implement for my real 'GetScriptForDevice' isn't always this easy. Any suggestions on a pattern or an example of a better way to structure this code to follow a better pattern would be greatly appreciated. I'm violating OCP every time I need to add a new DeviceScript and I'm trying to find a way around this because my class is getting quite long and fragile.
Thanks.
public class DeviceScript {
public string Name { get; set; }
public string SecurityScriptName { get; set; }
public string BusinessScriptName { get; set; }
}
public class DeviceAnalyzer {
public const string US_SecurityScript = "US_SecurityScript";
public const string US_BusinessScript = "US_BusinessScript";
public const string UK_SecurityScript = "UK_SecurityScript";
public const string UK_BusinessScript = "UK_BusinessScript";
private DeviceScript UnitedStates = new DeviceScript {
Name = "US_Script",
SecurityScriptName = US_SecurityScript,
BusinessScriptName = US_BusinessScript
};
private DeviceScript UnitedKingdom = new DeviceScript {
Name = "UK_Script",
SecurityScriptName = UK_SecurityScript,
BusinessScriptName = UK_BusinessScript
};
public DeviceScript GetScriptForDevice(IDevice device) {
if(device.Location == Locations.US) {
return UnitedStates;
}
if(device.Location == Locations.UK) {
return UnitedKingdom;
}
return null;
}
}Solution
I would suggest something like that
Now you can add more scripts just by adding new classes to your project. No xml configs are required :)
Futher suggestions:
public interface IDeviceScript
{
string Name { get; }
string SecurityScriptName { get; }
string BusinessScriptName { get; }
bool IsScriptApply(IDevice device);
}
public interface IDevice
{
Locations Location { get; }
// more methods and properties
}
[Serializable]
[AttributeUsage(AttributeTargets.Class)]
public class DeviceScriptAttribute : Attribute
{
}
[DeviceScript]
public class UnitedStatesScript : IDeviceScript
{
public string Name
{
get { return "US_Script"; }
}
public string SecurityScriptName
{
get { return "US_SecurityScript"; }
}
public string BusinessScriptName
{
get { return "US_BusinessScript"; }
}
public bool IsScriptApply(IDevice device)
{
return device.Location == Locations.US;
}
}
public IDeviceScript GetScriptForDevice(IDevice device)
{
IEnumerable deviceScripts =
Assembly.GetExecutingAssembly()
.GetTypes()
.Where(t => t.GetCustomAttributes().Any())
.Select(t => Activator.CreateInstance(t))
.Cast();
foreach (IDeviceScript deviceScript in deviceScripts)
{
if (deviceScript.IsScriptApply(device))
{
return deviceScript;
}
}
return null;
}Now you can add more scripts just by adding new classes to your project. No xml configs are required :)
Futher suggestions:
- After calculating deviceScripts enumerable once you can store it in some static field for further use.
- Name property of IDeviceScript could be moved to DeviceScriptAttribute if you don't need it in your code.
- I would make GetScriptForDevice method static but unfortunately .net doesn't support statics in interfaces :(
Code Snippets
public interface IDeviceScript
{
string Name { get; }
string SecurityScriptName { get; }
string BusinessScriptName { get; }
bool IsScriptApply(IDevice device);
}
public interface IDevice
{
Locations Location { get; }
// more methods and properties
}
[Serializable]
[AttributeUsage(AttributeTargets.Class)]
public class DeviceScriptAttribute : Attribute
{
}
[DeviceScript]
public class UnitedStatesScript : IDeviceScript
{
public string Name
{
get { return "US_Script"; }
}
public string SecurityScriptName
{
get { return "US_SecurityScript"; }
}
public string BusinessScriptName
{
get { return "US_BusinessScript"; }
}
public bool IsScriptApply(IDevice device)
{
return device.Location == Locations.US;
}
}
public IDeviceScript GetScriptForDevice(IDevice device)
{
IEnumerable<IDeviceScript> deviceScripts =
Assembly.GetExecutingAssembly()
.GetTypes()
.Where(t => t.GetCustomAttributes<DeviceScriptAttribute>().Any())
.Select(t => Activator.CreateInstance(t))
.Cast<IDeviceScript>();
foreach (IDeviceScript deviceScript in deviceScripts)
{
if (deviceScript.IsScriptApply(device))
{
return deviceScript;
}
}
return null;
}Context
StackExchange Code Review Q#25345, answer score: 4
Revisions (0)
No revisions yet.