This page shows the source for this entry, with WebCore formatting language tags and attributes highlighted.

Title

ABD: Improving the Aspect-modeling API for Quino

Description

<h>Overview</h> We discussed <abbr title="Always Be Designing">ABD</abbr> in a recent article <a href="{apps}view_article.php?id=458">ABD: Refactoring and refining an API</a>. To cite from that article, <bq>[...] the most important part of code is to think about how you’re writing it and what you’re building. You shouldn’t write a single line without thinking of the myriad ways in which it must fit into existing code and the established patterns and practices.</bq> With that in mind, I saw another teaching opportunity this week and wrote up my experience designing an improvement to an existing API. <h>Requirements</h> Before we write any code, we should know what we're doing.<fn> <ul> We use aspects (<c>IMetaAspects</c>) in Quino to add domain-specific metadata (e.g. the <c>IVisibleAspect</c> controls element visibility) Suppose we have such an aspect with properties A1...AN. When we set property A1 to a new value, we want to retain the values of properties A2...AN (i.e. we don't want to discard previously set values) The current pattern is to call <c>FindOrAddAspect()</c>. This method does what it advertises: If an aspect with the requested type already exists, it is returned; otherwise, an instance of that type is created, added and returned. The caller gets an instance of the requested type (e.g. <c>IVisibleAspect</c>). Any properties on the requested type that you want to change must have <i>setters</i>. If the requested type is an interface, then we end up defining our interface as mutable. Other than when building the metadata, every other use of these interfaces should not make changes. We would like to be able to define the interface as read-only (no setters) and make the implementation mutable (has setters). Code that builds the metadata uses both the interface and the implementation type. </ul> Although we're dealing concretely with aspects in Quino metadata, the pattern and techniques outlined below apply equally well to other, similar domains. <h>The current API</h> A good example is the <c>IClassCacheAspect</c>. It exposes five properties, four of which are read-only. You can modify the property (<c>OrderOfMagnitude</c>) through the interface. This is already not good, as we are forced to work with the implementation type in order to change any property other than <c>OrderOfMagnitude</c>. The current way to address this issue would be to make <i>all</i> of the properties settable on the interface. Then we could use the <c>FindOrAddAspect()</c> method with the <c>IClassCacheAspect</c>. For example, <code> var cacheAspect = Element.Classes.Person.FindOrAddAspect<iclasscacheaspect>( () => new ClassCacheAspect() ); cacheAspect.OrderOfMagnitude = 7; cacheAspect.Capacity = 1000; </code> For comparison, if the caller were simply creating the aspect instead of getting a possibly-already-existing version, then it would just use an object initializer. <code> var cacheAspect = Element.Classes.Person.Aspects.Add( new ClassCacheAspect() { OrderOfMagnitude = 7; Capacity = 1000; } } </code> This works nicely for creating the initial aspect. But it causes an error if an aspect of that type had already been added. Can we design a single method with all the advantages? <h>The new API</h> A good way to approach a new is to ask: How would we want the method to look if we were calling it? <code> Element.Classes.Person.SetCacheAspectValues( a => { a.OrderOfMagnitude = 7; a.Capacity = 1000; } ); </code> If we only want to change a single property, we can use a one-liner: <code> Element.Classes.Person.SetCacheAspectValues(a => a.Capacity = 1000); </code> Nice. That's even cleaner and has fewer explicit dependencies than creating the aspect ourselves. <h>Making it work for one aspect type</h> Now that we know what we want the API to look like, let's see if it's possible to provide it. We request an interface from the list of aspects but want to use an implementation to set properties. The caller has to indicate how to create the instance if it doesn't already exist, but what if it does exist? We can't just upcast it because there is no guarantee that the existing aspect is the same implementation. These are relatively lightweight objects and the requirement above is that the <i>property values</i> on the existing aspect are set on the returned aspect, not that the existing aspect is preserved. What if we just provided a mechanism for copying properties from an existing aspect onto the new version? <code> var cacheAspect = new ClassCacheAspect(); var existingCacheAspect = Element.Classes.Person.Aspects.FirstOfTypeOrDefault<iclasscacheaspect>(); if (existingCacheAspect != null) { result.OrderOfMagnitude = existingAspect.OrderOfMagnitude; result.Capacity = existingAspect.Capacity; // Set all other properties } // Set custom values cacheAspect.OrderOfMagnitude = 7; cacheAspect.Capacity = 1000; </code> This code does exactly what we want and doesn't require any setters on the interface properties. Let's pack this away into the API we defined above. The extension method is: <code> public static ClassCacheAspect SetCacheAspectValues( this IMetaClass metaClass, Action<classcacheaspect> setValues) { var result = new ClassCacheAspect(); var existingCacheAspect = metaClass.Aspects.FirstOfTypeOrDefault<iclasscacheaspect>(); if (existingCacheAspect != null) { result.OrderOfMagnitude = existingAspect.OrderOfMagnitude; result.Capacity = existingAspect.Capacity; // Set all other properties } setValues(result); return result; } </code> So that takes care of the boilerplate for the <c>IClassCacheAspect</c>. It hard-codes the implementation to <c>ClassCacheAspect</c>, but let's see how big a restriction that is once we've generalized below. <h>Generalize the aspect type</h> We want to see if we can do anything about generalizing <c>SetCacheAspectValues()</c> to work for other aspects. Let's first extract the main body of logic and generalize the aspects. <code> public static TConcrete SetAspectValues<tservice,>( this IMetaClass metaClass, Action<tconcrete,> copyValues, Action<tconcrete> setValues ) where TConcrete : new, TService where TService : IMetaAspect { var result = new TConcrete(); var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<tservice>(); if (existingAspect != null) { copyValues(result, existingAspect); } setValues(result); return result; } </code> <h>Remove constructor restriction</h> This isn't bad, but we've required that the <c>TConcrete</c> parameter implement a default constructor. Instead, we could require an additional parameter for creating the new aspect. <code> public static TConcrete SetAspectValues<tservice,>( this IMetaClass metaClass, <hl>Func<tconcrete> createAspect,</hl> Action<tconcrete,> copyValues, Action<tconcrete> setValues ) where TConcrete : TService where TService : IMetaAspect { var result = <hl>createAspect();</hl> var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<tservice>(); if (existingAspect != null) { copyValues(result, existingAspect); } setValues(result); return result; } </code> <h>Just pass in the new aspect to use</h> Wait, wait, wait. We not only don't need to the <c>new</c> generic constraint, we also don't need the <c>createAspect</c> lambda parameter, do we? Can't we just <i>pass in</i> the object instead of passing in a lambda to create the object and <i>then calling it immediately</i>? <code> public static TConcrete SetAspectValues<tservice,>( this IMetaClass metaClass, <hl>TConcrete aspect,</hl> Action<tconcrete,> copyValues, Action<tconcrete> setValues ) where TConcrete : TService where TService : IMetaAspect { var existingAspect = metaClass.Aspects.FirstOfTypeOrDefault<tservice>(); if (existingAspect != null) { copyValues(aspect, existingAspect); } setValues(aspect); return aspect; } </code> That's a bit more logical and intuitive, I think. <h>Redefine original method</h> We can now redefine our original method in terms of this one: <code> public static ClassCacheAspect SetAspectValues( this IMetaClass metaClass, Action<classcacheaspect> setValues) { return metaClass.UpdateAspect( new ClassCacheAspect(), (aspect, existingAspect) => { result.OrderOfMagnitude = existingAspect.OrderOfMagnitude; result.Capacity = existingAspect.Capacity; // Set all other properties }, setValues ); } </code> <h>Generalize copying values</h> Can we somehow generalize the copying behavior? We could make a wrapper that expects an interface on the <c>TService</c> that would allow us to call <c>CopyFrom(existingAspect)</c>. <code> public static TConcrete SetAspectValues<tservice,>( this IMetaClass metaClass, TConcrete aspect, Action<tconcrete> setValues ) where TConcrete : TService, ICopyTarget where TService : IMetaAspect { return metaClass.UpdateAspect<tservice,>( aspect, (aspect, existingAspect) => aspect.CopyFrom(existingAspect), setValues ); } </code> What does the <c>ICopyTarget</c> interface look like? <code> public interface ICopyTarget { void CopyFrom(object other); } </code> This is going to lead to type-casting code at the start of every implementation to make sure that the <c>other</c> object is the right type. We can avoid that by using a generic type parameter instead. <code> public interface ICopyTarget<t> { void CopyFrom(T other); } </code> That's better. How would we use it? Here's the definition for <c>ClassCacheAspect</c>: <code> public class ClassCacheAspect : IClassCacheAspect, ICopyTarget<iclasscacheaspect> { public void CopyFrom(IClassCacheAspect otherAspect) { OrderOfMagnitude = otherAspect.OrderOfMagnitude; Capacity = otherAspect.Capacity; // Set all other properties } } </code> Since the final version of <c>ICopyTarget</c> has a generic type parameter, we need to adjust the extension method. But that's not a problem because we already have the required generic type parameter in the outer method. <code> public static TConcrete SetAspectValues<tservice,>( this IMetaClass metaClass, TConcrete aspect, Action<tconcrete> setValues ) where TConcrete : TService, ICopyTarget<hl><tservice></hl> where TService : IMetaAspect { return metaClass.UpdateAspect( aspect, (aspect, existingAspect) => aspect.CopyFrom(existingAspect), setValues ); } </code> <h>Final implementation</h> Assuming that the implementation of <c>ClassCacheAspect</c> implements <c>ICopyTarget</c> as shown above, then we can rewrite the cache-specific extension method to use the new extension method for <c>ICopyTargets</c>. <code> public static ClassCacheAspect SetCacheAspectValues( this IMetaClass metaClass, Action<classcacheaspect> setValues) { return metaClass.UpdateAspect<iclasscacheaspect,>( new ClassCacheAspect(), setValues ); } </code> This is an extension method, so any caller that wants to use its own <c>IClassCacheAspect</c> could just copy/paste this one line of code and use its own aspect. <h>Conclusion</h> This is actually pretty neat and clean: <ul> We have a pattern where all properties on the interface are read-only We have a pattern where an aspect can indicate how its values are to be copied from another instance. This is basically boilerplate, but must be written only once per aspect---and it can be located right in the implementation itself rather than in an extension method. A caller building metadata passes in a single lambda to set values. Existing values are handled automatically. Adding support for more aspects is straightforward and involves very little boilerplate. </ul> <hr> <ft>You would think that would be axiomatic. You'd be surprised.</ft>