[stringtemplate-interest] IAttributeRenderer and Type Inheritance in C#

[Jonathan Buhacoff]

The existing ST code already works for subclasses.  My implementation  
of the interface renderer also works for subclasses that implement the  
same interface, but not because I did anything special - we get this  
from the language itself and from the renderer implementation, if it  
uses instanceof.

I like the idea of using isInterface to automatically separate the two  
kinds of renderers for later, but then the programmer can't control  
which interface renderer will apply to a value that implements more  
than one registered interface.

If the code were changed to add to a list instead of a map/dictionary,  
the programmer could control priority but then you have the same  
performance problem that you mentioned, plus the programmer will be  
forced to register interface-based handlers in order, which is not  
consistent with the current registration interface.

If it's implemented with a map or dictionary I would warn users not to  
use values that implement more than one registered interface because  
the behavior will not be predictable. This may be the simplest route.


--
Jonathan Buhacoff
jonathan at buhacoff.net
--



On Oct 23, 2009, at 6:52 PM, Sam Harwell wrote:

> This semi-solves one problem, but doesn’t address another at all.  
> What if you want to add a renderer for “[non-interface] type X and  
> all types derived from X”? Also, it still doesn’t address the  
> performance issue of the feature itself. Below is one way it could  
> be implemented without changing the public interface.
>
> Covered:
> ·         Performance when the feature is not in use
>
> ·         Registering renderers for interface type
>
> Not covered:
> ·         Performance implication when a renderer is registered for  
> an interface type
>
> ·         Registering renderers for a base type and having it also  
> apply to derived types
>
>
>
>     public virtual void RegisterRenderer( Type attributeClassType,  
> IAttributeRenderer renderer )
>     {
>         Dictionary<Type, IAttributeRenderer> renderers;
>         // renderers for interface types are kept separately due to  
> their performance implications
>         if (!attributeClassType.IsInterface)
>         {
>             if (_attributeRenderers == null)
>                 _attributeRenderers = new Dictionary<Type,  
> IAttributeRenderer>();
>
>             renderers = _attributeRenderers;
>         }
>         else
>         {
>             if (_interfaceRenderers == null)
>                 _interfaceRenderers = new Dictionary<Type,  
> IAttributeRenderer>();
>
>             renderers = _interfaceRenderers;
>         }
>
>         renderers[attributeClassType] = renderer;
>     }
>
>     public virtual IAttributeRenderer GetAttributeRenderer( Type  
> attributeClassType )
>     {
>         IAttributeRenderer renderer = null;
>         if ( _attributeRenderers != null )
>         {
>             if ( ! 
> _attributeRenderers.TryGetValue( attributeClassType, out renderer ) )
>                 renderer = null;
>         }
>
>         // Only need to perform the expensive interface checks if  
> the user registered a renderer for an interface type
>         if (renderer == null && _interfaceRenderers != null)
>         {
>             renderer = _interfaceRenderers.FirstOrDefault(pair =>  
> pair.Key.IsAssignableFrom(attributeClassType)).Value;
>         }
>
>         if ( renderer != null )
>         {
>             // found it!
>             return renderer;
>         }
>
>         // we have no renderer overrides for the template or none  
> for class arg
>         // check parent template if we are embedded
>         if ( _enclosingInstance != null )
>         {
>             return  
> _enclosingInstance.GetAttributeRenderer( attributeClassType );
>         }
>         // else check group
>         return _group.GetAttributeRenderer( attributeClassType );
>     }
>
>
> From: stringtemplate-interest-bounces at antlr.org [mailto:stringtemplate-interest-bounces at antlr.org 
> ] On Behalf Of Jonathan Buhacoff
> Sent: Friday, October 23, 2009 6:55 PM
> To: StringTemplate Mailing List
> Subject: Re: [stringtemplate-interest] IAttributeRendererand Type  
> Inheritancein C#
>
> Hi Sam,
>
> My last suggestion solves the performance and priority issues:
>
> A separate method for setting a list of interface renderers
>
> Something like:  public void setRenderers(List<AttributeRenderer>  
> renderers)
>
> Then in StringTemplate this list of renderers would be checked only  
> if a class renderer wasnt found.
>
> Performance issues would then only affect people using this feature.
>
>
>
>
>
> Sent from my iPhone
>
> On Oct 22, 2009, at 9:10 PM, "Sam Harwell" <sharwell at pixelminegames.com 
> > wrote:
>
> Hi Vincent and Jonathan,
>
> I can’t tell who said what here due to the way your editors  
> formatted the text. I thought about a possible solution to the  
> performance problems I mentioned, but I don’t think it will be  
> viable. I thought about always caching the results of the lookup, so  
> object interfaces would only need to be checked the first time a  
> type was used in a template. Unfortunately, due to the way templates  
> inherit attribute renderers, several problems emerge:
>
> 1.       Caching the renderers would result in a copy of all type- 
> >renderer maps in every template, incurring both a space and time  
> overhead.
>
> 2.       Caching the renderers would break the attribute renderer  
> inheritance scheme.
>
> 3.       Not caching the renderers would mean checking all interface  
> base types and interfaces while walking up the template inheritance  
> chain. This would be a more substantial hit than GetAttribute,  
> likely more than doubling the time required to render templates.  
> I’ll profile this to make sure, but if my analysis proves correct  
> the feature will have to be completely reengineered before we could  
> talk about incorporating it into the library.
>
>
> Sam
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.antlr.org/pipermail/stringtemplate-interest/attachments/20091025/09d2c25d/attachment-0001.html