tags:

views:

174

answers:

3

Hi folks,

this question is an extension to a previous question i asked (and was answered). I'm refactoring my code an playing around with / experimenting with various refactored solutions.

One of the solutions i came up with (but wasn't happy with .. remember, i'm just experimenting with some personal coding styles) wsa the following code :-

if (data is ITagElement)
{
    if (((ITagElement) data).TagList.IsNullOrEmpty())
    {
        ((ITagElement) data).TagList = new List<Tag>();
    }

    ((ITagElement) data).TagList.Add(new Tag
    {
        K = xmlReader.GetAttribute("k"),
         V = xmlReader.GetAttribute("v")
    });
}

Notice how i'm casting the parent object data to the interface type it impliments a number of times? Code works, but i feel like this is code smell -> that it's not very efficient. I feel like this could be cast improved upon - thoughts from any guru's out there?

+10  A: 

What about this, so you only do the cast once:

ITagElement someData = data as ITagElement
if (someData != null)
{
   if (someData.TagList.IsNullOrEmpty())
    {
        someData.TagList = new List<Tag>();
    }

    someData.TagList.Add(new Tag
    {
        K = xmlReader.GetAttribute("k"),
         V = xmlReader.GetAttribute("v")
    });

}
Tim Merrifield
Cheers! I like! :)
Pure.Krome
+2  A: 

I think it would be clearer as such:

if (data is ITagElement)
{
    ITagElement dataAsTagElement = (ITagElement)data;
    if (dataAsTagElement.TagList.IsNullOrEmpty())
    {
        dataAsTagElement.TagList = new List<Tag>();
    }

    dataAsTagElement.TagList.Add(new Tag
    {
        K = xmlReader.GetAttribute("k"),
        V = xmlReader.GetAttribute("v")
    });
}

This would be better as it's avoiding a cast 3 times in exchange for a pointer. It's also far easier on the eyes!

TheSoftwareJedi
+3  A: 

Use the as operator to get is and cast in the same op:

ITagElement tagElement = data as ITagElement;
if (tagElement == null) return;

if (tagElement.TagList.IsNullOrEmpty()) {
   tagElement.TagList = new List<Tag>();
}

tagElement.TagList.Add(new Tag
{
    K = xmlReader.GetAttribute("k"),
    V = xmlReader.GetAttribute("v")
});
Mark Brackett