2
\$\begingroup\$

I have implemented the structure below at least 5 times, and it is getting cumbersome. Is there a better approach?

Current code-segment implemented

public XmlNodeList Templates {
  get {
    return this.GetElementsByTagName( "Templates" );
  }
  set {
    foreach( UITemplateAssoc nd in value ) {
      this.AddTemplate( nd );
    }
  }
}
public UITemplateAssoc GetTemplate (string name) {
  foreach( UITemplateAssoc nd in this.Templates ) {
    if( nd.Name == name ) {
      return nd;
    }
  }

  return null;
}
public void AddTemplate ( UITemplateAssoc val ) {
  for( int i = this.Templates.Count - 1 ; i >= 0 ; i-- ) {
    if( this.Templates[i].Name == val.Name ) {
      this.ReplaceChild( val , this.Templates[i] );
      return;
    }
  }

  this.AppendChild( val );
}
\$\endgroup\$
2
\$\begingroup\$

There's a couple of nice alternatives for you.

Extensions

If you don't need to encapsulate the XmlNodeList instances, you could make your methods extensions for XmlDocument or whatever you've extended:

public static class TemplateExtensions
{
    public static XmlNodeList GetTemplates(this XmlDocument document) 
    {
        return document.GetElementsByTagName("Templates");
    }

    public static UITemplateAssoc GetTemplate(this XmlNodeList list, string name)
    {
        foreach(var nd in list)
        {
            if (nd.Name == name) return nd;
        }
    }

    // ...
}

// ...
var templates = yourDocument.GetTemplates();
var template = templates.GetTemplate("abc");

More on extensions methods here: http://msdn.microsoft.com/en-us/library/bb383977.aspx

Inheritance

Put these methods in a base class and have the behavior that differs in inherited classes.

[edit]Updated to show how the base class can be generic[/edit]

public abstract class TemplateListBase<T>
    where T : XmlNode
{
    private XmlNodeList templates;

    public XmlNodeList Templates 
    {       
        get 
        {       
            return templates.GetElementsByTagName( "Templates" );       
        }       
    }       

    protected TemplateListBase(XmlNodeList initialList)
    {
        templates = initialList;
    }

    public void AddRange(XmlNodeList list) 
    {       
        foreach( T nd in list ) 
        {       
            AddTemplate( nd );       
        }       
    }

    public T GetTemplate (string name) 
    {       
        foreach( T nd in templates ) 
        {       
            if( nd.Name == name ) return nd;       
        }
        return null;
    }       

    public void AddTemplate ( T val ) 
    {       
        for( int i = templates.Count - 1 ; i >= 0 ; i-- ) 
        {       
            if( templates[i].Name == val.Name ) 
            {       
                templates.ReplaceChild( val , templates[i] );       
                return;       
            }       
        }       
        templates.AppendChild( val );       
    }      
}

public class UITemplateAssocList : TemplateListBase<UITemplateAssoc>
{
    public UITemplateAssocList(XmlNodeList listOfUITemplates)
        : base(listOfUITemplates)
    {
    }

    public void FancyLogicA(string data)
    {
        foreach(var node in Templates)
        {
            // differing logic
        }
    }
}

public class ConcreteTemplateB : TemplateListBase<SomeOtherKindOfNode>
{
    // ..

    public void FancyLogicB(XmlNodeList list)
    {
        for (var node in list)
        {
            if (LikeThisNode(node)) AddTemplate(node);
        }
    }
}

Notice that I replaced your setter with an AddRange method. It's ususally good practice not to replace an entire encapsulated collection using a setter. Although you can validate the entries in the setter, a property should not do much logic other than returning or setting privates. If you have a look at the collections in the BCL, they all expose an AddRange method and a method Clear to empty the collection. The AddRange method also gives you the option of adding multiple sets in turn.

If you need different logic when stuff is added etc, have a look at the template method pattern: http://en.wikipedia.org/wiki/Template_method_pattern

If you need multiple base classes with differing logic, you could combine the inheritance with the extension methods, or another base class. :)

\$\endgroup\$
  • \$\begingroup\$ I already have several extensions at the base library for similar actions but they are foreseeable. Problem with this is that i really want to create a XMLNodeList that overrides the return type from the generic XmlNode to the specified class. not having much success though ;( \$\endgroup\$ – GoldBishop Feb 20 '12 at 18:26
  • \$\begingroup\$ Tried using generics? \$\endgroup\$ – Lars-Erik Feb 20 '12 at 18:45
  • \$\begingroup\$ yeah but couldnt get the pattern to overload the return values. Ran into a brick wall. Tried doing XmlNodeList<T>: XmlNodeList but XmlNodeList evidently is a final class and is not inheritable. \$\endgroup\$ – GoldBishop Feb 20 '12 at 19:04
  • \$\begingroup\$ Make a wrapper for XmlNodeList instead of inheriting it, or make the methods working with the elements generic. For instance public static T GetElement<T>(this XmlNodeList list, string name). Post some more code if you want, I'm sure it's possible. :) \$\endgroup\$ – Lars-Erik Feb 20 '12 at 20:00
  • \$\begingroup\$ I see I forgot to include the wrapping in the example. I've updated the example with a private instance of the XmlNodeList and made the base class generic for the items, not the list. Notice the base class does not inherit XmlNodeList, it wraps it. Unless stated that you're supposed to in the documentation, a rule of thumb is never to inherit stuff from the BCL. Much better to wrap. As an added bonus, you often get less dependencies in code using your class. \$\endgroup\$ – Lars-Erik Feb 20 '12 at 23:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.