OOP Analysis Design Mandatory Principles
Contents
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
- Class can be subclassed for extension
- Class' behaviors can be overriden
- No modification is on the locked behaviors of base class (related open behaviors aren't)
- 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:
- Allow the closed action might be overriden and reused on subclass
- 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 1
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.
Example 2
One more example. In this situation it's harder to see any really "repeating" code. But let's stare into the whole structure of classes, those were really ones, with the same bussiness repeating (it might become more and more if expanded later)
Original design
The mapping process - here is text mapping - was spreading due to the functionalities (representing by properties Func...).
Improved design
So, the design should have become
FreeLabelMapper & FreeLabelTextMapper
public class FreeLabelTextMapper : FreeLabelMapper
{
/// <summary>
/// Gets or sets function after having text mapped
/// </summary>
public override Func<FreeLabelItemStyle, FreeLabelItemStyle> MappingFunc
{
get
{
return item =>
{
if (Replacer == null || Replacer.Count == 0 || !(Replacer.First() is string))
goto Return;
item.Text = item.Text.Replace(Key, (string) Replacer.First());
Return : return item;
};
}
}
}
/// <summary>
/// Free label mapping class
/// </summary>
public class FreeLabelMapper
{
#region Properties
/// <summary>
/// Gets or sets key of the mapper
/// </summary>
public string Key { get; set; }
/// <summary>
/// Gets or sets replacing objects
/// </summary>
public IList<object> Replacer { get; set; }
/// <summary>
/// Gets or sets function after mapped by the <see cref="FreeLabelMapper"/>
/// </summary>
public virtual Func<FreeLabelItemStyle, FreeLabelItemStyle> MappingFunc { get; protected set; }
#endregion
}
FreeLabelHandler
public IList<FreeLabelTextMapper> TextMappers { get; protected set; }
internal FreeLabelHandler()
{
// DCS last build time
//TryAddMapper(StyleConst.DCSLastBuildTimeExp, DataCacheLastBuildTimeMapper);
TextMappers = new List<FreeLabelTextMapper>();
}
internal FreeLabelHandler(DiagramInfo diagramInfo, Domain.Reports.Diagram diagram)
: this()
{
Diagram = diagram;
DiagramInfo = diagramInfo;
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.DCSLastBuildTimeExp,
Replacer = new[] { string.Format("{0} : {1}",
DataCacheLastBuildTimeText,
DiagramInfo.DataCache.LastBuildInfo.LastBuildEnd) }
});
Validate();
}
/// <summary>
/// Get a collection of transformed free labels from predefined rule
/// </summary>
/// <param name="additionalFunc">Additional function to filter free label items once</param>
/// <returns>Free label item collection</returns>
internal virtual void Transform(Func<FreeLabelItemStyle, FreeLabelItemStyle> additionalFunc)
{
RetrieveFreeLabels();
var freeLabels = new FreeLabelItemCollectionStyle();
foreach (FreeLabelItemStyle item in FreeLabels)
{
var clone = item;
foreach (var textMapper in TextMappers)
clone = textMapper.MappingFunc(clone);
freeLabels.Add(clone);
}
FreeLabels = freeLabels;
}
TrackingDiagramFreeLabelHandler
internal TrackingDiagramFreeLabelHandler(DiagramInfo trackingDiagramInfo, Diagram diagram)
: base(trackingDiagramInfo, diagram)
{
var trackingInfo = (TrackingDiagramInfo) DiagramInfo;
// Start date time
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.FromWeekExp,
Replacer = new[] { string.Format("{0}/{1}", trackingInfo.TimePeriod.StartDateTime.Week,
trackingInfo.TimePeriod.StartDateTime.Year) }
});
// End date time
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.ToWeekExp,
Replacer = new[] { string.Format("{0}/{1}", trackingInfo.TimePeriod.EndDateTime.Week,
trackingInfo.TimePeriod.EndDateTime.Year) }
});
// Time period
var tp = trackingInfo.TimePeriod;
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.TimePeriodExp,
Replacer = new[] { string.Format("{0} : {1} {2}/{3} - {1} {4}/{5}",
TimePeriodText, WeekText, tp.StartDateTime.Week, tp.StartDateTime.Year,
tp.EndDateTime.Week, tp.EndDateTime.Year) }
});
// Average respondents
// TODO
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.AverageRespondentsExp,
Replacer = new[] { string.Empty }
});
// Warning low base
// TODO
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.WarningLowBase,
Replacer = new[] { string.Empty }
});
// Target group
// TODO
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.TargetGroupExp,
Replacer = new[] { string.Empty }
});
// Sub-title
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.SubtitleExp,
Replacer = new[] { trackingInfo.Expressions != null && trackingInfo.Expressions.Length > 0 &&
trackingInfo.Expressions[0].Length == 1 && !string.IsNullOrEmpty(trackingInfo.Expressions[0][0].Alias)
? trackingInfo.Expressions[0][0].Alias
: string.Empty }
});
// Calculation methods
TextMappers.Add(new FreeLabelTextMapper
{
Key = StyleConst.CalculationMethodsExp,
Replacer = new[] { string.Format("{0} : {1} {2}", CalculationMethodsText,
((TrackingDiagramInfo) DiagramInfo).TimePeriod.RollingAverage, WeekText) }
});
}
As the result, any mapping process - here is text mapping - only takes place in only one place : Overriding MappingFunc of FreeLabelTextMapper
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:
- Where ? Spot multiple responsibilities
- 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
