3

I am having difficulty figuring out an efficient (thread-safe) code design for the following problem. I have been at it for some time now and would really appreciate some advice and input on how best to approach this.

I am basically searching for various patterns inside a data set. I would like to have one "master" class. I call this class Patterns. I am thinking of making this class a singleton pattern or a static class. I wish to have only one instance of this master class. I am coding this class as a singleton pattern (pattern by Jon Skeet) at this time.

 public sealed class Patterns
 {
      private Patterns()
      {}

      public static Patterns Instance { get { return Nested.instance; } }

      private class Nested
      {
           // Explicit static constructor to tell C# compiler
           // not to mark type as beforefieldinit
           static Nested()
           {
                // ...  create a list to hold data for internal use
                List<Point> Data = new List<Point>();
           }

           internal static readonly Patterns instance = new Patterns();
      }
 }

I have chosen a singleton pattern because:

  1. I want the singleton to hold nested classes; one class for each pattern.
  2. I want these nested classes to be instance classes (i.e. not static)
  3. I would like the singleton to instantiate the nested classes, perhaps using a factory pattern, as needed.

I am not sure whether my reasoning here is correct so any advice would be greatly appreciated.

Now I plan to add the various pattern classes as nested classes because patterns do not make sense outside the master Pattern class. Also, IPattern is an interface for the patterns.

 public sealed class Patterns
 {
      private Patterns()
      {}

      public static Patterns Instance { get { return Nested.instance; } }

      private class Nested
      {
           // Explicit static constructor to tell C# compiler
           // not to mark type as beforefieldinit
           static Nested()
           {
                // ...  create a list to hold data for internal use
                static List<Point> Data = new List<Point>();

                // ... indices to track occurences of each pattern
                static int AIndex = 0;
                static int BIndex = 0;
                static int CIndex = 0;
           }

           internal static readonly Patterns instance = new Patterns();
      }

      // ... Methods

      public bool SearchForPattern(Point dataPoint)
      {
           // ... add data to list
           Data.Add(dataPoint);
      }

      // ... nested pattern classes

      class PatternA : IPattern
      {
           static int = some integer
           static double[] arr = new double[];

           // ... do some work
      }

      class PatternB : IPattern
      {
           // ... do some work
      }

      class PatternC : IPattern
      {
           // ... do some work
      }
 }

The List Data is static because I want the data to be retained throughout the lifetime of the application. Same for the static fields in the nested classes.

This is where I now begin to really get lost. In the method SearchForPattern(Point dataPoint) I would like to do the following:

      public bool SearchForPattern(Point dataPoint)
      {
           // ... add data to list
           Data.Add(dataPoint);

           bool haveA = PatternA.SearchForA();
           if (haveA)
           {
                PatternA a1 = new PatternA();     // the number after a will change as other A patterns are found; as per AIndex value
                AIndex++;
           }

                ... and so on for other patterns
      }

So here are my questions. Please note that I am fairly new to C# and I am trying to learn.

  1. Is this the best (i.e. most computationally efficient) approach to take with this problem? If not, can you please suggest an alternative approach?

  2. I do not know how to incorporate the factory pattern for the instantiation of the nested pattern classes. Should this be done? If so, can you suggest how?

  3. Am I missing anything here that I need to consider but have not considered so far?

  • singleton is a bad idea, it is ever not needed or will lead to trouble. – ctrl-alt-delor Oct 12 '14 at 19:49
  • 4
    Thank you for your reply but this is not helpful to me. You do not say why in my case and what would be an alternative. Furthermore, there are many arguments on the net that singletons are useful. – Zeos Oct 12 '14 at 19:52
  • 1
    The most important reason — Singleton violates “single responsibility principle”: singleton is a class and its factory rolled into one. The less important reason — Singleton in nearly impossible to implement, at there is almost always a way around it. – ctrl-alt-delor Oct 12 '14 at 20:09
  • 1
    Please note comments are not answers, therefore they do not answer the question. – ctrl-alt-delor Oct 12 '14 at 20:09
  • I don't have an answer, except to say I think you may be on the wrong path. singleton is considered harmful, god/master class is considered harmful, making a library of patterns is considered harmful: Some patterns can be libryfied in some languages, but in general they can not. Pattern seek to go beyond libraries. There are reusable ideas, where as libraries are reusable code. – ctrl-alt-delor Oct 12 '14 at 20:15
5

I have chosen a singleton pattern because:

  1. I want the singleton to hold nested classes; one class for each pattern.

This doesn't require a singleton, or a static class. You could have

public class Patterns // non-static
{
    public class PatternA : IPattern { }
    public class PatternB : IPattern { }
}

and you'd be able to use it as a bag for your pattern definitions, and to instantiate instances of PatternA and PatternB just the same.

  1. I want these nested classes to be instance classes (i.e. not static)

This again has nothing to do with using a singleton (see above).

  1. I would like the singleton to instantiate the nested classes, perhaps using a factory pattern, as needed.

Once again, no singleton necessary. You could have a non-static pattern factory:

public class PatternsFactory
{
    public IEnumerable<IPattern> CreatePatterns()
    {
        return new List<IPattern> 
        {
            new Patterns.PatternA(), 
            new Patterns.PatternB()
        };
    }
}

Now I plan to add the various pattern classes as nested classes because patterns do not make sense outside the master Pattern class.

And why not? That's what namespaces are for. Using an umbrella class as a bag for class definitions looks like reinventing the wheel to me. Namespaces serve this very purpose: grouping related classes together. Why bounce them out of their only job?

If patterns may make no sense outside the Patterns namespace, mark them as internal, that is invisible from outside the namespace. You might want to keep IPattern itself public for testing purposes, though (about this in a while).

Am I missing anything here that I need to consider but have not considered so far?

One of the reasons that the use of statics is frowned upon is testability. How would you write a suite of unit tests for your pattern matching functionality? You can't create a mock pattern, because you cannot inject another pattern into your Pattern class. Heck, you can't even subclass it, since you made it sealed.

You're forcing all patterns to save their matches in a static Data object, which means that testing would require wiping it clean after every test. And again, there is no way to override it, other than rewriting the thing.

More reading on the subject:

Another concern is extensibility. What happens every time you want to add another pattern? You need to edit your Patterns class and put another implementation in there. You need to edit code of SearchForPattern to add all these nice

if (haveN) PatternN n = new PatternN();

That's inflexible and hard to maintain. The open/closed principle states that code should be open for extension, and your approach rules this out.

If not, can you please suggest an alternative approach?

I would ditch singletons, because I see no purpose of using them. I'd create a PatternFactory, capable of returning a set of all supported IPattern objects, and instantiate the factory wherever I would need it (it's stateless anyway).

As for the data - pattern matches - that you want to retain throughout the lifetime of the application (List Data is static because I want the data to be retained throughout the lifetime of the application), well, you can make it static, although that's not the only possible way of tying its lifecycle to the one of the application.

But why be hellbent on placing it in the same class object where all the patterns are defined?? Knowing all supported patterns, and persisting all pattern matches that we detected somewhere are not the same responsibility at all (see Single Responsibility Principle). This design choice smells of the God object antipattern.

Is this the best (i.e. most computationally efficient) approach to take with this problem?

What is computationally expensive here is the pattern matching itself, for non-trivial amounts of data at least. Therefore the cost of creating instances as opposed to using a static class is negligible, so as far as OOP design goes, I wouldn't expect performance to be affected.

  • 1
    Thank you for the detailed and thoughtful answer Konrad. It is interesting that currently I have the various pattern classes inside a namespace. I was simply looking for a way to centralize the patterns and searching for them. You have given me much to think about. Thank you. – Zeos Oct 12 '14 at 22:05
  • @Zeos happy to hear this, glad I could help – Konrad Morawski Oct 12 '14 at 22:06
  • Thank you for the detailed and thoughtful answer Konrad. It is interesting that currently I have the various pattern classes inside a namespace. I was simply looking for a way to centralize the patterns and searching for them. You have given me much to think about; I clearly need to learn more about code design. Thank you. Also, can you please elaborate on your second last paragraph? What is the other way to static? – Zeos Oct 12 '14 at 22:12
  • Also, I understand your last paragraph to mean that I should have one object for detecting the patterns and another one for persisting them throughout the lifecycle of the application. Can you suggest which objects? Do you mean a Dictionary to hold the patterns found? Thank you very much. – Zeos Oct 12 '14 at 22:12
  • If you need to keep track of which matches were detected by which pattern, then yes you could have a Dictionary with various IPattern as keys, or maybe even a dedicated PatternMatch class (consisting of an IPattern and a collection of matches), which could be more convenient in the long run - see programmer.97things.oreilly.com/wiki/index.php/… The recommended choice would depend on the broader context. The important thing is that these solutions do not require knowing a precise, finite list of available patterns beforehand. – Konrad Morawski Oct 12 '14 at 22:28

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.