OOP Analysis Design Mandatory Principles

<accesscontrol>Main:MyGroup</accesscontrol>

Open & Closed Principle (OCP)

Introduction

As the name itself, it's a principle instruct us how to close the main base behaviors of the original classes by preventing almost direct modifications, also to open existing classes to extend or make some changes by subclass and override on them.

Description

1. Class can be subclassed for extension
   
2. Class' behaviors can be overriden
   
3. No modification is on the locked behaviors of base class (related open behaviors aren't)
   
4. Modifications are on subclass
   

Specification

• Description #1 and #2 means that base class must assure the accessibilities and inheritance
• Description #3 and #4 means that we have 2 methods for the base class extension:

1. Allow the closed action might be overriden and reused on subclass
2. Allow some actions open on base class so that those actions might happen inside the closed action

Application

Having been applied spreadly in new Report engine, especially when we have been brought highest priorities onto close inheriting and reusage

Example

Structure

DiagramInfo as the base:

• CopyStaticDataFrom is wanted to be standard and unchangable in diagram generating logic: Validating DiagramType of input Diagram
  
• CopyStaticDataFrom is wanted to be extended in diagram generating logic in setting: Stylesheet, PredefinedCriterias, PredefinedLayouts, hidden filters
  

/// <summary>
      /// Set predefined criteria due to chart's specific characteristic
      /// <param name="diagram"></param>
      /// </summary>
      protected virtual void SetPredefinedCriteria(Diagram diagram) { /* No operation performed */ }

      /// <summary>
      /// Set predefined layout due to chart's specific characteristic
      /// <param name="diagram"></param>
      /// </summary>
      protected virtual void SetPredefinedLayout(Diagram diagram) { /* No operation performed */ }

      /// <summary>
      /// Set style sheet with corresponding properties
      /// </summary>
      /// <param name="diagram"></param>
      protected virtual void SetStyleSheet(Diagram diagram)
      {
         ChartLayoutInfo.SetStyleSheet(diagram.Style ?? StyleSheet.GetDefault());
      }

      /// <summary>
      /// Set hidden filters
      /// </summary>
      /// <param name="diagram"></param>
      protected virtual void SetHiddenFilters(Diagram diagram) { /* No operation performed */ }

      /// <summary>
      /// Copy some specific static data from a <see cref="Diagram"/> to <see cref="DiagramInfo"/>, 
      /// appropriete to type of diagram. Basic action is to set <see cref="StyleSheet" /> of <see cref="ChartLayoutInfo"/>
      /// </summary>
      /// <param name="diagram">Diagram owning data to be copied</param>
      public virtual void CopyStaticDataFrom(Diagram diagram)
      {
         if(diagram.Type != DiagramType)
            throw new ArgumentException("Diagram must be {0} type", DiagramType.ToString());

         SetStyleSheet(diagram);
         SetPredefinedCriteria(diagram);
         SetPredefinedLayout(diagram);
         SetHiddenFilters(diagram);
      }

CrossDiagramInfo as a subclass

Has Hidden filters needing specific action

/// <summary>
      /// Set hidden filters
      /// </summary>
      /// <param name="diagram"></param>
      protected override void SetHiddenFilters(Diagram diagram)
      {
         // Appy hidden filters
         var filter = HiddenFilter.GetHiddenFilter(DataAccess.AccessFactory.CurrentUser, diagram);
         if (filter != null)
            Filters = (Filters.Union(new[] {new FilterInfo(diagram.DataCacheSpecification, filter.Filter)})).ToArray();
      }

TrackingDiagramInfo as a subclass

Has Stylesheet, PredefinedCriterias, PredefinedLayout needing specific actions

/// <summary>
      /// Set predefined criterias, this includes some accessory conditions
      /// </summary>
      protected override void SetPredefinedCriteria(Diagram diagram)
      {
         // TODO : Implement further features like:
         // - Generated automatically as default on loading viewer page
         // - Hide criteria panel right as default on loading viewer page
         if (!SelfDefinedCriterias.ContainsKey(SelfDefinedCriteriaType.Tracking_AutoGenerated))
            SelfDefinedCriterias.Add(SelfDefinedCriteriaType.Tracking_AutoGenerated, false);
         else
            SelfDefinedCriterias[SelfDefinedCriteriaType.Tracking_AutoGenerated] = false;

         if (!SelfDefinedCriterias.ContainsKey(SelfDefinedCriteriaType.Tracking_CriteriaHidden))
            SelfDefinedCriterias.Add(SelfDefinedCriteriaType.Tracking_CriteriaHidden, false);
         else
            SelfDefinedCriterias[SelfDefinedCriteriaType.Tracking_CriteriaHidden] = false;
      }

      /// <summary>
      /// Set features of predefined layouts
      /// </summary>
      protected override void SetPredefinedLayout(Diagram diagram)
      {
         if (ChartLayoutInfo != null)
         {
            // Tracking always excluded total column calculation. This added here as the only one place. DO NOT CHANGE, Dude !
            if (!ChartLayoutInfo.SelfDefinedLayouts.ContainsKey(SelfDefinedLayoutType.Tracking_TotalCalculationExcluded))
               ChartLayoutInfo.SelfDefinedLayouts.Add(SelfDefinedLayoutType.Tracking_TotalCalculationExcluded, true);
            else
               ChartLayoutInfo.SelfDefinedLayouts[SelfDefinedLayoutType.Tracking_TotalCalculationExcluded] = true;
         }
      }

      /// <summary>
      /// Set style sheet with FreeLabelCollection of BarLine chart
      /// </summary>
      protected override void SetStyleSheet(Diagram diagram)
      {
         base.SetStyleSheet(diagram);

         if (FreeLabelHandler != null)
         {
            FreeLabelHandler.Transform(null);
            ChartLayoutInfo.StyleSheet.BarLineChartStyle.FreeLabelCollections = FreeLabelHandler.FreeLabels;
         }
         // Specific : Stylesheet - Free labels
         FreeLabelHandler =
            new TrackingDiagramFreeLabelHandler(this, diagram, ChartLayoutInfo.StyleSheet.BarLineChartStyle.FreeLabelCollections);
      }

      /// <summary>
      /// Copy some specific static data from a <see cref="Diagram"/> to <see cref="TrackingDiagramInfo"/>, appropriete to type of diagram
      /// </summary>
      /// <param name="diagram">Diagram owning data to be copied</param>
      public override void CopyStaticDataFrom(Diagram diagram)
      {
         base.CopyStaticDataFrom(diagram);
         
         // Static chart title
         ChartTitle = diagram.ChartTitle;
      }

Don't Repeat Yourself Principle (DRY)

Introduction

Main purpose of this principle is to avoid duplicated code. Benefits of this principle: Easy to maintain code and assure flow of system not to run into different branches but they should have been the only one.

Description

To do that, it follows the slogan: One requirement in One place.

Specification

Slogan in description section means not only trying to collect as many features & requirements as possible to put in one place, but also making good decisions on system's functionalities.

Application

We have serious duplicated code problem in bunches of place of the old Report engine. That leads dispersion : When fixing/changing one, we must always find to fix/change in too many other places. So the old system is really hard to maintain and always relied on longtemp experience and manual attempt than on close structure and favorable design decision.

With new Report system, now whenever any change/addition required, there's always consideration to avoid code duplication as much as possible, based on clear design in which classes and their subclass are close to each other and have the tightest relationship.

Example

Lets come back to the example of OCP, in TrackingDiagramInfo methods SetPredefinedCriteria, SetPredefinedLayout we saw the code that duplicated the same feature in 3 times, though they have different logic. Now I want to make sure that I will implement the feature one single time in the only single place. By this way, I can reserve for further case in which this setting attempt may happen. Where I oughta determine to put this ?

In TrackingDiagramInfo ? No, as it should have been taken inside the root methods of DiagramInfo. So it must be in a helper file of DiagramInfo. I named it as CommonHelper.

SetPredefinedCriteria

/// <summary>
      /// Set predefined criterias, this includes some accessory conditions
      /// </summary>
      protected override void SetPredefinedCriteria(Diagram diagram)
      {
         // TODO : Implement further features like:
         // - Generated automatically as default on loading viewer page
         // - Hide criteria panel right as default on loading viewer page
         SelfDefinedCriterias.TrySet(SelfDefinedCriteriaType.Tracking_AutoGenerated, false);
         SelfDefinedCriterias.TrySet(SelfDefinedCriteriaType.Tracking_CriteriaHidden, false);
      }

SetPredefinedLayout

/// <summary>
      /// Set features of predefined layouts
      /// </summary>
      protected override void SetPredefinedLayout(Diagram diagram)
      {
         if (ChartLayoutInfo != null)
         {
            // Tracking always excluded total column calculation. This added here as the only one place. DO NOT CHANGE, Dude !
            ChartLayoutInfo.SelfDefinedLayouts.TrySet(SelfDefinedLayoutType.Tracking_TotalCalculationExcluded, true);
         }
      }

SetPredefinedCriteriaExtension (in CommonHelper)

/// <summary>
   /// An extension class for <see cref="SelfDefinedCriteriaType"/>
   /// </summary>
   public static class SelfDefinedCriteriaTypeExtension
   {
      /// <summary>
      /// Try to set a object-typed value with corresponding <see cref="SelfDefinedCriteriaType"/>-typed key no matter what any item owning this key has existed or not
      /// </summary>
      /// <param name="dictionary">Dictionary has item whose key to be set value</param>
      /// <param name="key">Item key to be set</param>
      /// <param name="value">Value of item</param>
      /// <returns>Dictionary has item whose value already set</returns>
      public static Dictionary<SelfDefinedCriteriaType, object> TrySet(this Dictionary<SelfDefinedCriteriaType, object> dictionary, SelfDefinedCriteriaType key, object value)
      {
         if (!dictionary.ContainsKey(key))
            dictionary.Add(key, value);
         else
            dictionary[key] = value;
         return dictionary;
      }
   }

SetPredefinedLayoutExtension (in CommonHelper)

/// <summary>
   /// An extension class for <see cref="SelfDefinedLayoutType"/>
   /// </summary>
   public static class SelfDefinedLayoutTypeExtension
   {
      /// <summary>
      /// Try to set a object-typed value with corresponding <see cref="SelfDefinedLayoutType"/>-typed key no matter what any item owning this key has existed or not
      /// </summary>
      /// <param name="dictionary">Dictionary has item whose key to be set value</param>
      /// <param name="key">Item key to be set</param>
      /// <param name="value">Value of item</param>
      /// <returns>Dictionary has item whose value already set</returns>
      public static Dictionary<SelfDefinedLayoutType, object> TrySet(this Dictionary<SelfDefinedLayoutType, object> dictionary, SelfDefinedLayoutType key, object value)
      {
         if (!dictionary.ContainsKey(key))
            dictionary.Add(key, value);
         else
            dictionary[key] = value;
         return dictionary;
      }
   }

Note

• SetPredefinedCriteriaExtension and SetPredefinedLayoutExtension seems the same to each other because DRY here can't be applied anymore as the limitation of an extension class
  
• When hidden filters are determined to be applied in TrackingDiagramInfo, it's easy to see that hidden filter applying process of Cross, in fact, is the very general one. So we may move CrossDiagramInfo applying hidden filter code up to DiagramInfo, then remove the one in CrossDiagramInfo & unnecessary to add this into TrackingDiagramInfo. That's another DRY

Single Responsibility Principle (SRP)

Introduction

SRP is a principle used to assure that objects should be simplestand most concrete at much as possible.

That means that we try to implement an object with exact features it should own, avoid its non-instinctive function.

Description

The object should be splitted into objects handle exactly what operations they should take.

Further than DRY, the question becomes not only "Can we collect the physical duplication into one place ?", but as well "Can we reduce duplication by separating the business duplication out of its object"

Specification

To apply above description, there are steps:

1. Where ? Spot multiple responsibilities
   
2. Then How ? Look into the operations/properties and move it to right location/split it if possible. Then come back to step #1
   

Application

In new Report engine

Example

ExpressionResolver is a common class. TrackingDiagramInfo was using it directly, as properties called OriginalExpression and Expressions.

/// <summary>
/// Gets or sets the original expression to build up the usage expressions
/// </summary>
public ExpressionResolver[] OriginalExpression { get; set; }

/// <summary>
/// Gets or sets an relating <see cref="ExpressionResolver"/> collection for further free-label calculation, 
/// to Reserved for further free-label handling, as method SetStyleSheet of TrackingDiagramInfo unabled to receive 
/// outer ExpressionResolvers to initialize free label hanlder. This is necessary for Free label implementation. 
/// It's just the transitting data so unnecessary to serialize it
/// </summary>
[Common.Serialization.NonSerialized]
public ExpressionResolver[][] Expressions { get; set; }

These were initialized by Javascript

Type.registerNamespace("CatGlobe.Framework.Report.Diagrams.ExpressionResolver");

CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver = function(alias, value)
{
   this.__type = "CatGlobe.Framework.Report.Diagrams.ExpressionResolver, CatGlobe.Report";
   this.Alias = alias;
   this.Value = value;
}

CatGlobe.Framework.Report.Diagrams.ExpressionResolver.registerClass("CatGlobe.Framework.Report.Diagrams.ExpressionResolver", null);

if (childNodes.length == 0)
      {
         itms.push(new CatGlobe.Framework.Report.Diagrams.ExpressionResolver(itemNode.getAttribute("name"), itemNode.getAttribute("value")));
      }
      // Including the only one 2-level brand
      else
      {
         for (var childIndex = 0; childIndex < childNodes.length; childIndex++)
         {
            var childItemNode = childNodes[childIndex];
            itms.push(new CatGlobe.Framework.Report.Diagrams.ExpressionResolver(childItemNode.getAttribute("name"), childItemNode.getAttribute("value")));
         }
      }

and Json converter

/// <summary>
      /// <see cref="ExpressionResolverJavaScriptConverter"/> convert from/to simple string as expression to DynamicExpressionResolver
      /// </summary>
      private class ExpressionResolverJavaScriptConverter : AbstractDiagramInfoDependentConverter<ExpressionResolver>
      {
         private const string Alias = "Alias";
         private const string Value = "Value";

         public ExpressionResolverJavaScriptConverter(DataCacheSpecification dataCache)
            : base(dataCache)
         { }

         protected override IEnumerable<string> SerializableProperties
         {
            get
            {
               return new[] { Alias, Value };
            }
         }

         protected override Func<IDictionary<string, object>, JavaScriptSerializer, ExpressionResolver> Create
         {
            get
            {
               return (dict, serializer) =>
                      new ExpressionResolver((string)dict[Value], new CGScriptCalculator(_dataCache))
                         {
                            Alias = (string) dict[Alias]
                         };
            }
         }
      }

and also used for UnitTest

Besides Value is already indicated as the most basic one of a brand expression, when Base began to be needed, it should be considered where to implement.

Firstly Base was considered to be put in ExpressionResolver. But in fact Base is not a exact instinctive property of ExpressionResolver. Just think about ExpressionResolver, not only applied for Tracking/Campaign, but also for Cross/Standard, then why Cross/Standard must have Base when they don't need that feature ? This could lead a further unncessary bulky mess, such as UnitTest for Cross/Standard.

Then we decided to create a new class called TrackingDiagramExpressionResolver - inheriting from ExpressionResolver - having ability to handle the specific features of Tracking (later should be CampaignDiagramExpressionResolver) like Base (and more ... ?).

/// <summary>
   /// Specific expression resolver for brands of Tracking diagram
   /// </summary>
   public class TrackingDiagramExpressionResolver : ExpressionResolver
   {
      #region Variables

      private string _base;
      private int _parentIndex = -1;

      #endregion

      #region Constructors

      public TrackingDiagramExpressionResolver(string value, ICalculator calculator)
         : base(value, calculator)
      { }

      #endregion

      #region Properties

      /// <summary>
      /// Gets or sets raw Base of a brand option
      /// </summary>
      public string Base
      {
         get { return _base; }
         set
         {
            var val = value.Trim().TrimEnd(';');
            if (string.IsNullOrEmpty(_base) || !_base.Equals(val)) _base = val;
         }
      }

      /// <summary>
      /// Gets or protected set calculated Base from raw Base of a brand option
      /// </summary>
      public object CalculatedBase { get; protected set; }

      /// <summary>
      /// Gets or sets index of parent option. This is used for further tracing
      /// </summary>
      public int ParentIndex
      {
         get { return _parentIndex; }
         set { _parentIndex = value; }
      }

      #endregion

      #region Methods

      /// <summary>
      /// Evaluate all values need calculating, including <see cref="Base"/>
      /// </summary>
      public override void Evaluate()
      {
         base.Evaluate();

         CalculatedBase = Calculator != null && !string.IsNullOrEmpty(Base) ? Calculator.Execute(Base) : Base;
      }

      #endregion
   }

Then TrackingDiagramInfo's OriginalExpression and Expressions initialization by Javascript becomes

Type.registerNamespace("CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver");

CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver = function(alias, value, base, parentIndex)
{
   this.__type = "CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver, CatGlobe.Report";
   this.Alias = alias;
   this.Value = value;
   this.Base = base;
   this.ParentIndex = parentIndex;
}

CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver.registerClass("CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver", null);

...

 if (childNodes.length == 0)
      {
         itms.push(new CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver(itemNode.getAttribute("name"), itemNode.getAttribute("value"), itemNode.getAttribute("base"), -1));
      }
      // Including the only one 2-level brand
      else
      {
         for (var childIndex = 0; childIndex < childNodes.length; childIndex++)
         {
            var childItemNode = childNodes[childIndex];
            itms.push(new CatGlobe.Framework.Report.Diagrams.TrackingDiagramExpressionResolver(childItemNode.getAttribute("name"), childItemNode.getAttribute("value"), childItemNode.getAttribute("base"), index));
         }
      }

and Json converter becomes

/// <summary>
      /// <see cref="TrackingDiagramExpressionResolverJavaScriptConverter"/> convert from/to simple string as expression to DynamicExpressionResolver
      /// </summary>
      private class TrackingDiagramExpressionResolverJavaScriptConverter : AbstractDiagramInfoDependentConverter<TrackingDiagramExpressionResolver>
      {
         private const string Alias = "Alias";
         private const string Value = "Value";
         private const string Base = "Base";
         private const string ParentIndex = "ParentIndex";

         public TrackingDiagramExpressionResolverJavaScriptConverter(DataCacheSpecification dataCache)
            : base(dataCache)
         { }

         protected override IEnumerable<string> SerializableProperties
         {
            get
            {
               return new[] { Alias, Value, Base, ParentIndex };
            }
         }

         protected override Func<IDictionary<string, object>, JavaScriptSerializer, TrackingDiagramExpressionResolver> Create
         {
            get
            {
               return (dict, serializer) =>
                      new TrackingDiagramExpressionResolver((string)dict[Value], new CGScriptCalculator(_dataCache))
                         {
                            Alias = (string) dict[Alias], 
                            Base = (string) dict[Base],
                            ParentIndex = (int) dict[ParentIndex]
                         };
            }
         }
      }

and UnitTest changed also