views:

118

answers:

4

I'm currently struggling to understand how I should organize/structure a class which I have already created. The class does the following:

  1. As its input in the constructor, it takes a collection of logs
  2. In the constructor it validates and filters the logs through a series of algorithms implementing my business logic
  3. After all filtering and validation is complete, it returns a collection (a List) of the valid and filtered logs which can be presented to the user graphically in a UI.

Here is some simplified code describing what I'm doing:

class FilteredCollection
{
  public FilteredCollection( SpecialArray<MyLog> myLog)
  {   
  // validate inputs
  // filter and validate logs in collection
  // in end, FilteredLogs is ready for access
  }
  Public List<MyLog> FilteredLogs{ get; private set;}

}

However, in order to access this collection, I have to do the following:

var filteredCollection = new FilteredCollection( specialArrayInput );
//Example of accessing data
filteredCollection.FilteredLogs[5].MyLogData;

Other key pieces of input:

  1. I foresee only one of these filtered collections existing in the application (therefore should I make it a static class? Or perhaps a singleton?)
  2. Testability and flexibility in creation of the object is important (Perhaps therefore I should keep this an instanced class for testability?)
  3. I'd prefer to simplify the dereferencing of the logs if at all possible, as the actual variable names are quite long and it takes some 60-80 characters to just get to the actual data.
  4. My attempt in keeping this class simple is that the only purpose of the class is to create this collection of validated data.

I know that there may be no "perfect" solution here, but I'm really trying to improve my skills with this design and I would greatly appreciate advice to do that. Thanks in advance.


EDIT:

Thanks to all the answerers, both Dynami Le-Savard and Heinzi identified the approach I ended up using - Extension Methods. I ended up creating a MyLogsFilter static class

namespace MyNamespace.BusinessLogic.Filtering
{
    public static class MyLogsFilter
    {
        public static IList<MyLog> Filter(this SpecialArray<MyLog> array)
        {
            // filter and validate logs in collection
            // in end, return filtered logs, as an enumerable
        }
    }
}

and I can create a read only collection of this in code by doing

IList<MyLog> filteredLogs = specialArrayInput.Filter(); 
ReadOnlyCollection<MyLog> readOnlyFilteredLogs = new ReadOnlyCollection<MyLog>(filteredLogs);
+1  A: 

Some thoughts:

  • As you correctly point out, using an instanced class improves testability.

  • Singletons should be used if (A) there is only one instance of the class in your whole system and (B) you need to access this instance at multiple different places of your application without having to pass the object around. Unnecessary use of the Singleton pattern (or any other kind of "global state") should be avoided, so unless (B) is satisfied in your case as well, I'd not use a singleton here.

  • For simple dereferencing, consider using an indexer. This will allow you to write:

    FilteredCollection filteredlogs = new FilteredCollection( secialArrayInput );
    //Example of accessing data
    filteredlogs[5].MyLogData;
  • If your class only consists of a constructor and a field to access the result, using a simple method might be more appropriate than using a class. If you want to do it the fancy way, you could write it as an extension method for SpecialArray<MyLog>, allowing you to access it like this:
    List<MyLog> filteredlogs = secialArrayInput.Filter();
    //Example of accessing data
    filteredlogs[5].MyLogData;
Heinzi
Thank you for pointing out the benefits/detriments of the singleton pattern. This is a great list of options. I hope to do more than simple dereferencing with the object created, so I think the extension method is the way to go. The only question I would have would be - is it appropriate to return something like a "read only" list of filtered logs? Or is that overkill, and I should just trust future programmers to not change the list once its created?
CrimsonX
I think I answered my own question - I can force the list to be read only if I wish by doing this: `IList<MyLog> filteredLogs = specialArrayInput.Filter();``ReadOnlyCollection<MyLog> readOnlyFilteredLogs = new ReadOnlyCollection<MyLog>(filteredLogs);`
CrimsonX
+3  A: 

It sounds like you do three things to your logs:

  1. Validate them
  2. Filter them and
  3. Access them

You want to store the logs in a collection. The standard List collection is a good fit since it doesn't care what's in it, gives you LINQ and allows you to lock the collection with a read-only wrapper

I would suggest you separate your concerns into the three steps above.

Consider

interface ILog
{
  MarkAsValid(bool isValid);
  ... whatever data you need to access...
}

Put your validation logic in a separate interface class

interface ILogValidator
{
  Validate(ILog);
}

And your filtering logic in yet another

interface ILogFilter
{
  Accept(ILog);
}

Then with LINQ, something like:

List<MyLog> myLogs = GetInitialListOfLogsFromSomeExternalSystem();
myLogs.ForEach(x => MyLogValidator(x));
List<MyLog> myFilteredLogs = myLogs.Where(x => MyLogFilter(x));

The separation of concerns makes testing and maintainability much better. And stay away from the singletons. For many reasons including testability they are out of favor.

Rob
Thank you for suggesting an alternative design using interfaces! The overall filtering of the collection is actually dependent on the order and interrelationships between individual log objects, so an operation like `myLogs.ForEach(x => MyLogValidator(x));` wouldn't work in my case. You did however remind me that this code is *just* doing the filtering exercise, so I'll properly divide the filtering/validation in the future.
CrimsonX
As a follow up question - you mention 'The standard List collection is a good fit since it doesn't care what's in it, gives you LINQ and allows you to lock the collection with a read-only wrapper'. I know I can do something like `Public List<MyLog> FilteredLogs{ get; private set;}` when its within a class, but is there any other way to lock a collection when returning a collection from a method call? (I figure there is not)
CrimsonX
I think I answered my own question - I can force the list to be read only if I wish by doing this: `IList<MyLog> filteredLogs = specialArrayInput.Filter(); ReadOnlyCollection<MyLog> readOnlyFilteredLogs = new ReadOnlyCollection<MyLog>(filteredLogs);`
CrimsonX
A: 

If you want to inherit the interface of SpecialArray for you final filtered array then derive from SpecialArray instad of having an instance member. That would allow:
filteredCollecction[5].MyLogData; etc..

Ricibob
+1  A: 

The way I see it, you are looking at a method that returns a collection of filtered log, rather than a collection class wrapping your business logic. Like so:

class SpecialArray<T>
{
     [...]

     public IEnumerable<T> Filter()
     {   
         // validate inputs
         // filter and validate logs in collection
         // in end, return filtered logs, as an enumerable
     }

     [...]
}

However, it does look like what you really wish is actually to separate the business logic in charge of filtering the logs from the SpecialArray class, perhaps because you feel like the logic touches many things that do not really concern SpecialArray, or because Filter does not apply to all generic cases of SpecialArray.

In that case my suggestion would be to isolate your business logic in another namespace, perhaps one that uses and/or requires other components in order to apply said business logic, and offer your functionality as an extension method, concretly :

namespace MyNamespace.Collections
{
    public class SpecialArray<T>
    {
        // Shenanigans
    }
}

namespace MyNamespace.BusinessLogic.Filtering
{
    public static class SpecialArrayExtensions
    {
        public static IEnumerable<T> Filter<T>(this SpecialArray<T> array)
        {
            // validate inputs
            // filter and validate logs in collection
            // in end, return filtered logs, as an enumerable
        }
    }
}

And when you need to use that business logic, it would look like this :

using MyNamespace.Collections; // to use SpecialArray
using MyNamespace.BusinessLogic.Filtering; // to use custom log filtering business logic
namespace MyNamespace
{
    public static class Program
    {
        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        static void Main2()
        {
            SpecialArray<Logs> logs;
            var filteredLogs = logs.Filter();
        }
    }
}
Dynami Le Savard
Awesome explanation of the utility of using an extension method! You correctly realized that for me, "Filter" does not apply to all generic cases of SpecialArray. As of right now I've created a extension method with a signature `public static IList<MyLog> Filter(this SpecialArray<MyLog> myLog)` and it appears to be working properly. The only question I would have would be - is it appropriate to return something like a "read only" list of filtered logs? Or is that overkill, and I should just trust future programmers to not change the list once its created?
CrimsonX
I think I answered my own question - I can force the list to be read only if I wish by doing this: `IList<MyLog> filteredLogs = specialArrayInput.Filter(); ReadOnlyCollection<MyLog> readOnlyFilteredLogs = new ReadOnlyCollection<MyLog>(filteredLogs);`
CrimsonX