views:

162

answers:

6

I have my domain model with several NewsType's which are subclasses of NewsItem as shown below (simplified):

public abstract class NewsItem : Entity
{
    public virtual Account Account { get; set; }
    public virtual DateTime DateTime { get; set; }
}

Here are a couple of subclasses of NewsItem:

public class NewsItemJoiner : NewsItem
{
    public virtual Account AccountJoined { get; set; }
}

public class NewsItemStatus : NewsItem
{
    public virtual string Status { get; set; }
}

In my MVC app I want to return a collection of Newsitem's which may contain many different subclasses of NewsItem. Is what I now need to do is loop through each news item and call a Render function from the relevant class for that specific type of NewsItem...code might explain it a little easier:

public interface IRenderer<T> where T : NewsItem
{
    string Render(T item);
}

public class JoinedRenderer : IRenderer<NewsItemJoiner>
{
    public string Render(NewsItemJoiner item)
    {
        return String.Format("{0} has just joined our music network.", item.AccountJoined.ArtistName);
    }
}

public class StatusUpdateRenderer : IRenderer<NewsItemStatus>
{
    public string Render(NewsItemStatus item)
    {
        return String.Format("<span class='statusupdate'>{0}<span>", item.Status);
    }
}

I need to somehow call the correct classes Render function depending on the type of NewsItem.

+2  A: 

You could make a Dictionary that used the type of NewsItem as a key and the Render function to be used as a value. Or, you could maintain a list of all of the classes with Render functions or just of all the Render functions and use Reflection to determine which method should be used. However, it seems to me that instead of doing any of this you should consider redesigning your application so that the NewsItem abstract class itself has a virtual Render function. This would greatly simplify your task.

Edit: Previously thought NewsItem was an interface.

Brett Widmeier
+2  A: 

This seems like a rather obvious case for a virtual function.....

public abstract class RenderableNewsItem : NewsItem
{
    abstract public string Render();
}

public class NewsItemStatus : RenderableNewsItem 
{ 
    public virtual string Status { get; set; } 
    public string Render() 
    { 
        return String.Format("<span class='statusupdate'>{0}<span>", this.Status); 
    } 
}
James Curran
That would be true if the real usage was as simple as the OP's example, but that's not often the case on SO.
Ben M
+1  A: 

One possibility: on startup (i.e. in a static constructor related to your rendering code), iterate through the classes in your assembly and instantiate and store a Dictionary<Type, object> of IRenderer<T>-implementing instances mapped to the type that they render.

(This suggestion assumes that the renderer objects are thread-safe, since you may end up calling the Render method from more than one request thread at one time. If they're not thread safe, then you'd need to change the dictionary to <Type, Type> and instantiate a renderer for each use.)

For example:

public class RenderUtil
{
    static Dictionary<Type, object> s_renderers;

    static RenderUtil()
    {
        s_renderers = new Dictionary<Type, object>();

        foreach (var type in Assembly.GetExecutingAssembly().GetTypes())
        {
            var renderInterface = type.GetInterfaces().FirstOrDefault(
                i => i.IsGenericType && 
                     i.GetGenericTypeDefinition() == typeof(IRenderer<>));

            if (renderInterface != null)
            {
                s_renderers.Add(
                    renderInterface.GetGenericArguments()[0],
                    Activator.CreateInstance(type));
            }
        }
    }

    public static string Render<T>(T item)
    {
        IRenderer<T> renderer = null;
        try
        {
            // no need to synchronize readonly access
            renderer = (IRenderer<T>)s_renderers[item.GetType()];
        }
        catch
        {
            throw new ArgumentException("No renderer for type " + item.GetType().Name);
        }

        return renderer.Render(item);
    }
}

Usage:

var newsItem = new NewsItemStatus();

// in your example code, ends up calling StatusUpdateRenderer.Render:
var rendered = RenderUtil.Render(newsItem);

Note that the RenderUtil class will throw a DuplicateKeyException via a TypeInitializationException on first use if there is more than one renderer for a given type.

Ben M
I like this implementation and I have just tried it out...it's nearly working except that it's looking for a Renderer of type NewsItem instead of the correct subclass such as JoinerNewsItem. I am using NHibernate to return my NewsItem's and they are declared as NewsItem even though they contain the additional properties of each subclass...i need to somehow downcast them to the correct type. I know downcasting isn't safe or best practice but I think this is the only way? or am i missing something. Thank you!!
Paul Hinett
Ok i solved this issue by using a dictionary of type Dictionary<string, object>, instead of storing the object type i store the name of the object instead using GetType().Name.ToString()Thank you very much for the answer!
Paul Hinett
Glad to be of help. (I've changed the `Render` method to show another way of fixing the problem you encountered.)
Ben M
A: 

Consider inverting the control logic and providing a virtual Render() method in NewsItem instead. E.g.

abstract class NewsItem {
    // ...
    public virtual string Render() { return string.Empty; }
}

Then your subclass can implement as desired:

public class NewsItemJoiner : NewsItem
{
    // ...
    public override string Render() {
        return String.Format("{0} has just joined our music network.", this.AccountJoined.ArtistName);
    }
}

Edit:
Alternative Technique
Point taken about the comments from others re separation of concerns. I don't know if you're set on the IRenderer<T> for other reasons, but if you aren't, there's another technique which doesn't need to use reflection. You could use the Visitor pattern instead.

First you declare a NewsItemVisitor class:

public abstract class NewsItemVisitor
{
    public abstract void Visit(NewsItemJoiner joiner);
    public abstract void Visit(NewsItemStatus status);
}

Next, add a virtual Accept() method to NewsItem (for this example, I've changed your data types to string instead of Account, Status etc):

public abstract class NewsItem
{
    public virtual string Account { get; set; }
    public virtual DateTime DateTime { get; set; }
    public abstract void Accept(NewsItemVisitor visitor);
}

public class NewsItemJoiner : NewsItem
{
    public virtual string AccountJoined { get; set; }
    public override void Accept(NewsItemVisitor visitor)
    {
        visitor.Visit(this);
    }
}

public class NewsItemStatus : NewsItem
{
    public virtual string Status { get; set; }
    public override void Accept(NewsItemVisitor visitor)
    {
        visitor.Visit(this);
    }
}

Now you can create a concrete Visitor, which is our renderer:

public class NewsItemListRenderer : NewsItemVisitor
{
    private readonly List<NewsItem> itemList;
    private string renderedList = string.Empty;

    public NewsItemListRenderer(List<NewsItem> itemList)
    {
        this.itemList = itemList;
    }

    public string Render()
    {
        foreach (var item in itemList)
        {
            item.Accept(this);
        }

        return renderedList;
    }

    public override void Visit(NewsItemJoiner joiner)
    {
        renderedList += "joiner: " + joiner.AccountJoined + Environment.NewLine;
    }

    public override void Visit(NewsItemStatus status)
    {
        renderedList += "status: " + status.Status + Environment.NewLine;
    }
}

Sample code for how to render a list of NewsItem instances:

List<NewsItem> itemList = new List<NewsItem>();
itemList.Add(new NewsItemJoiner { AccountJoined = "fred" });
itemList.Add(new NewsItemJoiner { AccountJoined = "pete" });
itemList.Add(new NewsItemStatus { Status = "active" });
itemList.Add(new NewsItemJoiner { AccountJoined = "jim" });
itemList.Add(new NewsItemStatus { Status = "inactive" });

NewsItemListRenderer renderer = new NewsItemListRenderer(itemList);
Console.WriteLine(renderer.Render());

Running this gives the following output:

joiner: fred
joiner: pete
status: active
joiner: jim
status: inactive
RichTea
This kind of breaks the whole idea of MVC, no?
Ben M
It does, but I also think it's the cleanest solution to the problem. Architecting a complicated solution with lookup tables and everything else that's been suggested seems (debatably, to be sure) silly just to enforce MVC.
Sapph
This was my initial design, however I don't beleive my domain model should be returning formatted HTML. My IRenderer is at my controller level.
Paul Hinett
The cleanest solution to the problem is not always the best when you're intentionally separating responsibilities between code layers (i.e., MVC).
Ben M
I'd have categorised IRenderer with view rather than controller though ...
RichTea
A: 

I just did something like this, but I don't have the code handy. It used Reflection and looked like the following:

List<GenericContainer> gcList = new List<GenericContainer>();
// GenericContainer can be a Jug, Bottle, Barrel, or just a GenericContainer type
// [..fill it..]
GenericContainer gc = gcList[i];
Object returnvalue = gc.GetType()
                     .GetMethod("Pour", BindingFlags.Instance).Invoke(gc, null);
clintp
+1  A: 

What I would do instead is have multiple partial views for rendering different NewsItem subclasses. Then, I would have some sort of mapping between the subclasses and the partial view names. Here's two ways to do this:

  1. NewsItem could have a virtual string property/method that returns the name of the partial view associated with it. I'd only recommend this if NewsItem is a specifically a model class used to pass into views, not if it's an ORM class or similar.
  2. In the model containing the list of news items, you could have a mapping property (a Dictionary<Type, string> for example).

Once you have this set up, your view could look something like this:

<% foreach (var newsItem in Model.NewsItems) { %>
    <%= Html.RenderPartial(newsItem.PartialViewName, newsItem) %>
<% } >
Jacob
This is another option i may look into once i have a little more time, however I am using strongly typed view models mapped with AutoMapper which may add a little complication. The reason why i like this is that i can have much more control over the format of the HTMl outputted using partial views.
Paul Hinett