Hi Paul,

I just read the article and here are some things that I would have done differently. That's doesn't mean it's a better :-)

Use MVC convetions for viewpath

I would have created the view in a folder ~/Views/SiteLayout/. Then you can do and MVC will pick up the path because the view is located in a folder with the name of the controller

return PartialView("_TopNavigation.cshtml", nav);

Query content instead of using path level

You use a fixed level for your homepage and then use the path to determine it. I would have just queried the content to get the homepage.

IPublishedContent homePage = this.Umbraco.AssignedContentItem.AncestorOrSelf("HomepageDocTypeAlias");

Use strongly typed content access instead of dynamic

Your method to get childitems takes a dynamic parameter page. I would have used strongly typed IPublishedContent

Then you can do you visible check like this

var childPages = page.Children.Where(x => x.IsVisible()).ToList();

Also you have some redudant checks in this method : childPages.Any() && childPages.Count > 0

Read this article about the difference between dynamic and stronlgy typed content access : http://24days.in/umbraco/2015/strongly-typed-vs-dynamic-content-access/

Caching

For data caching I would have used built in caching from Umbraco : https://our.umbraco.org/documentation/Reference/Cache/updating-cache

If the menu doesn't need a active state I would probably have used (donut) outputcache only. So it's cached once for the entire website.

If the navigation needed a active state that would have used both memory and output cache.

1. Add/Retreive menu structure from cache
2. Set active state
3. Output cache so it's only rendered once

Another thing is also I don't see anything to clear your cache when a editor makes changes in the backend.

That's my input. Hope you will find it usefull

Thanks Paul for this interesting blog. Very helpful.

However, it may sound stupid, but how would you go about adding related links for example home, blogs etc.. using cache implementation for navigation model.

One small thing I noticed was that you had a redundant check on this property on the NavigationListItem class:

public bool HasChildren { get { return Items != null && Items.Any() && Items.Count > 0;  } }

You don't need the && Items.Count > 0 after calling Items.Any() as it will never be evaluated and is redundant.

Also, you can use the built-in Umbraco classes in ApplicationContext.ApplicationCache.RuntimeCache for caching items rather than rolling your own.

is working on a reply...