5

I am creating some software that runs on a machine that performs multiple tests on a product that is built at my company. I am also trying to improve my coding and I have been recently researching design patterns and seeing how I can apply them to my code.

The part I am currently looking at is the actual test sequence. At the moment my code looks similar to this

public class TaskItem
{
  public string TaskName {get;set;}
  public DateTime StartTaskDateTime {get;set;}
  public DateTime EndTaskDateTime {get;set;}
  public Funct<bool> TaskFunction {get;set;}

  public bool Execute()
  { 
    StartDateTime = DateTime.Now;
    var taskResult = TaskFunction.Invoke();
    EndDateTime = DateTime.Now;
    return taskResult;
  }
}

Then my class that builds the sequence and runs it is like this

List<TaskItem> TestTaskItems = new List<TaskItem>();

public void BuildTestSquence()
{
    TestTaskItems.Add(new TaskItem()
    {
       TaskFunction = () =>
       { 
          // Do some Stuff
       }
    };

    // ... add more task items
}

public bool RunTestSequnce()
{
   foreach(var taskItem in TestTaskItems)
   {
      if(!taskItem.Execute())
      {
        return false;
      }
   }

   return true;
}

As the BuildTestSquence method could be adding anywhere between 1 task to 100 tasks, you can image that the method could have a lot of code in it. In my actual code I current have 400 lines of code in that method. I am just wondering if what I have done is really the right way of doing it.

I was thinking maybe the Chain of Responsibility might be an idea, and if it is then I am not too sure on the best way to implement it. From what I have seen so far, most examples show that you pass in an object and each chain does something with this object before deciding whether to pass it on or not. I won't be passing in any object.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
Gaz83
  • 275
  • 3
  • 9
  • 3
    The design pattern you are already using is called "command pattern". That is fine. Now the remaining question is how to build a list of objects in a more efficient manner. – Doc Brown Jan 29 '17 at 18:17
  • You said "list", but does it matter what order the tests are executed in? Can they be executed in parallel? – Kevin Krumwiede Jan 29 '17 at 19:18
  • Have you considered using NUnit or other test framework? This would give you a de facto code pattern that is list-based, sounds like what you are looking for. – John Wu Jan 29 '17 at 21:46
  • 1
    Possible duplicate of [Choosing the right Design Pattern](http://softwareengineering.stackexchange.com/questions/227868/choosing-the-right-design-pattern) –  Feb 01 '17 at 23:58

2 Answers2

4

There is no design pattern that I know of to accomplish this.

But you can improve your code (reduce the size of your BuildTestSequence() function) by writing each TaskFunction as a regular method and then passing it to your newly created TaskItem as follows:

TestTaskItems.Add( new TaskItem()
    {
        TaskFunction = TaskFunction1
    } );

or, better yet, as follows:

TestTaskItems.Add( new TaskItem( TaskFunction1 ) )

(assuming you have added TaskFunction as a parameter to the constructor of TaskItem.)

A further improvement is to populate your TestTaskItems collection programmatically, using reflection, so as to further reduce the size of your BuildTestSquence() function and to ensure that you did not forget to include any Task Functions.

Begin by doing GetType() on the class which contains BuildTestSquence() and RunTestSequnce(), and from the returned Type get all non-static methods. Then, you have a number of ways for deciding whether a method is a Task Function: You can mandate that the names of all of your Task Functions begin with a certain prefix, or, (the best way,) you can declare your own "Attribute" (lookup C# attributes) and add it to your Task Functions, so that if you find a method that has this attribute, you know it is a Task Function.

Mike Nakis
  • 32,003
  • 7
  • 76
  • 111
  • Similarly, I was going to suggest using MEF to resolve tests at runtime. – MetaFight Jan 29 '17 at 16:54
  • Thanks for that. You have given me something to think about :-) – Gaz83 Jan 29 '17 at 17:14
  • Glad to be of help. I noticed you have asked questions in the past which have received decent answers but you have not accepted any of them, so I cannot help but remark that the convention here is to "accept" the answer that best suits your needs. (Not immediately, but after a couple of days have passed.) – Mike Nakis Jan 29 '17 at 17:57
  • 2
    Good answer, found this example for the reflection approach: http://stackoverflow.com/questions/12323633/any-design-patterns-for-initialization-c-sharp – Doc Brown Jan 29 '17 at 18:23
  • @MikeNakis Just looking further into your idea of building the collection using reflectiong. How do you handle functions that require different parameters? Example, I have `Method 1` that require 2 `int` parameters and `Method 2` requires a `string` and a `bool`. How do use reflection and then supply the parameters that is required? If it helps, the class that has the `BuildTestSequence` has the relevant properties that are passed onto these methods as parameters. – Gaz83 Jan 31 '17 at 19:58
  • There is no easy answer to that. But how did you work with TaskItems in the TestTaskItems collection? You appear to have had a `TaskFunction.Invoke();` method, which obviously takes no arguments, so you appear to have already resolved this issue somehow. – Mike Nakis Jan 31 '17 at 21:50
  • TaskFunction returned bool so it was something like TaskFunction = () => { return Method1(someIntProperty, someIntProperty2); }. – Gaz83 Jan 31 '17 at 22:22
2

To me having a hardcoded BuildTestSequence function containing the code to run is bad design.

If this function is hardcoded, ie you have the actual code that you run listed there in. Then it would be better to simply rename that function RunTestSequence and have it execute the steps.

The only justification for having a such a function is where you are de-serialising the steps to call from a data-store of some sort.(or reflecting them out) Then your build function is generic and only a few lines of code.

Even then I would consider executing the step immediately before moving onto the next unless you can benefit from parallelisation, as you may needless use up a lot of memory building the list of tasks.

Ewan
  • 70,664
  • 5
  • 76
  • 161