3
\$\begingroup\$

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.

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

}
\$\endgroup\$
3
  • \$\begingroup\$ Use indirection like an XML config file (such as app settings) that can store mapping between a key and a class name. Ultimately I see no logic (e.g. code that does something) in your classes; just data storage/mapping, so you could define all the behavior in a simple Excel file, which you can then save as a CSV file and load it dynamically as your app starts. Another alternative would be storing your table in a tiny embedded database and read stuff out of it. Your problem does not seem to need an OOP solution. All you need is one big table from what I can tell. \$\endgroup\$
    – Leonid
    Apr 22, 2013 at 17:31
  • \$\begingroup\$ For example: stackoverflow.com/questions/1883884/… stackoverflow.com/questions/9791393/… \$\endgroup\$
    – Leonid
    Apr 22, 2013 at 18:09
  • 1
    \$\begingroup\$ DeviceScript should be a interface. You would then extend DeviceScript for both US and UK (and others as they come up). Then you'd probably want to use a factory to return the correct DeviceScript based on Location and other factors. and also you want to make a empty script to so you don't return null. \$\endgroup\$ Apr 22, 2013 at 20:39

2 Answers 2

4
\$\begingroup\$

I would suggest something like that

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

Now you can add more scripts just by adding new classes to your project. No xml configs are required :)

Futher suggestions:

  1. After calculating deviceScripts enumerable once you can store it in some static field for further use.
  2. Name property of IDeviceScript could be moved to DeviceScriptAttribute if you don't need it in your code.
  3. I would make GetScriptForDevice method static but unfortunately .net doesn't support statics in interfaces :(
\$\endgroup\$
3
  • \$\begingroup\$ +1 Replacing a manual mapping--whether it be XML, INI, .txt, .yml or C# code--with a simple string concatenation is always a win. When that's not possible, go with an external file that does the mapping over a switch that must be updated by a developer. \$\endgroup\$ Apr 23, 2013 at 3:47
  • \$\begingroup\$ +0 = -0 from me. While this answer is better than the original, having to change and recompile the code every time a data change needs to be made is not the "winner" in my mind. \$\endgroup\$
    – Leonid
    Apr 23, 2013 at 16:43
  • \$\begingroup\$ @Leonid you don't have to recompile your code to add new classes if you implement some sort of plugin mechanism (ex MEF msdn.microsoft.com/en-us/magazine/ee291628.aspx) \$\endgroup\$ Apr 23, 2013 at 19:34
1
\$\begingroup\$

Building on @user1614543's answer, you could create a more generic implementation of IDeviceScript:

public class DeviceScript : IDeviceScript
{
   public DeviceScript (string name, string securityScriptName, string businessScriptName, Locations supportedLocation)
   {
      Name = name;
      SecurityScriptName = securityScriptName;
      BusinessScriptName = businessScriptName;
      Location = supportedLocation;
   }

   public string Name { get; private set; }
   public string SecurityScriptName { get; private set; }
   public string BusinessScriptName { get; private set; }
   public bool IsScriptApply (IDevice device)
   {
       return device.Location == Location;
   }

   private Locations Location { get; set; }
}

You would still use the visitor pattern he proposed, but this alternative IDeviceScript implementation would allow you to populate your list of available device scripts via some configuration source (a flat file, database, app.config, etc.).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.