Bringing down the Hammer
I mentioned in Part 1 that DotLiquid's Condition hierarchy could do with being a bit more object oriented.
As conditions are a relatively small and isolated part of the API, it's a great place to start this series in earnest, so that's where I'll begin.
The Restructure
Here's a before and after of the Condition class hierarchy.
BEFORE
AFTER
First, I introduced a new interface, ICondition, and I did this for two reasons:
- Not all future developers will want to use the class ConditionBase as a base - they might have new code requirements or their own base class.
- No class that has a dependency on conditions should be forced to depend upon a specific implementation - by using the interface I make those classes compatible with any implementation.
Next, I refactored And and Or logic out of Expression and into their own classes. I did this because the code for And, Or and Expression may be logically cohesive, but it is not functionally cohesive. Incidentally, their code's lack of functional cohesion is what made them so easy to separate.
I made ConditionBase an abstract class to better indicate its purpose as a foundation, as opposed to a class that can be used effectively on its own.
I moved the static collection Operators out of ExpressionCondition and into its own class. This needs further work, as it shouldn't be static at all, but it's a start. More on this in a later post.
The IsElse property is a classic code smell because it will only be true on one occasion: when the Type is ElseCondition. Any logic that utilises the property would be better off inside the ElseCondition itself, thereby encapsulating the functionality, so I changed the signature of the Evaluate method to take a ConditionalStatementState object and moved the check for whether an ElseCondition should render inside ElseCondition.
// BEFORE
// =====================
// The owning block's render method:
var executeElseBlock = true;
foreach (var block in Blocks)
{
if (block.IsElse)
{
if (executeElseBlock)
{
return RenderAll(block.Attachment, context, result);
}
}
else if (block.Evaluate(context))
{
RenderAll(block.Attachment, context, result);
executeElseBlock = false;
}
}
// The ElseCondition's evaluate method:
public override bool Evaluate(Context context)
{
return true;
}
// AFTER
// =====================
// The owning block's render method:
var state = new ConditionalStatementState(context);
foreach (var block in Blocks)
{
if (block.Evaluate(state))
{
++state.BlockRenderCount;
var retCode = block.Render(context, result);
if (retCode != ReturnCode.Return)
return retCode;
}
}
// The ElseCondition's evaluate method:
public override bool Evaluate(ConditionalStatementState state)
{
return state.BlockRenderCount <= 0;
}
It's worth noting that I could have introduced an additional base class for AndCondition and OrCondition for which they override the evaluate method and share the Left and Right properties, but they do so little internally that it felt like overkill. Should they ever grow in size, an abstract base class can be retrofitted painlessly enough.
Summary
Overall, this is a great first step on the path to a clean and pure API, but there's still a lot more work to be done. I suspect that by the end of this series DotLiquid's API will be a significantly different beast, exposing the same functionality in a much more flexible API.
I'm really enjoying the challenge and, if you'd like me to clarify anything, feel free to let me know in the comments!
