A Quick Code Review with the Benevolent Orchard Dictator Emeritus

Tuesday, September 2, 2014 7:04:49 PM

I spent an hour with Bertrand Le Roy on the phone today. This cost me some money and was worth it. His facial expressions alone told me much of value about my code. I’m a lone developer.  As such, I work mostly in a vacuum, so what I wanted was:

  1. Code Review
  2. Development Workflow Review
  3. Three or so areas of improvement on which to focus for the next three months or so.

Here are some notes on my take home message from the call. These notes may well only be useful to me. They are here for future reference:

There are three valid ways to render a projection/query in a custom shape layout.

  1. render the content item
    1. increased control
    2. decreased flexibility
  2. use the @Display(shape) method
    1. increased reusability through alternates
    2. clean inversion of control through placement.info
    3. this is the canonical though not necessarily the best way
    4. if we’re going to do it this way, though, then why not just use one of the build in query layouts instead of a custom shape layout.
  3. use the @Display(shape.PartName), to create highly specific zones, then target those zones through placement
    1. this is the middle ground
    2. it has less flexibility than rendering the contentItem
    3. it is easier to understand, because of it has less inversion of control than does using @Display(shape)

Orchard uses deferred execution to create shapes from DriverResults.

  1. drivers use lambdas
  2. this delays execution
  3. Orchard will only execute the lambda if needed (e.g. placement)
  4. I.e. the driver result contains instructions for how to build a shape, and Orchard will later decide whether to run those instructions.
  5. so, just because we call a driver, doesn’t necessarily mean that we’re going to be making a database call

Ways to handle the limitations of projections & queries.

  1. whenever possible, create queries in the Orchard UI and leverage the built-in layouts
  2. if you have a more complex query with unusual sorts/groupings, then take one of two approaches
    1. for smaller numbers of items, it’s just barely acceptable to group/sort them in memory – try to do the sorting/grouping in a controller, though, not in a view, and do it with as few database calls as possible
    2. for larger numbers of items, use a controller action and build the query with HQL – this will make things like pagination more effecient
  3. note that sorting/grouping on field values can be hard if the field lacks an indexing table
  4. also note that the default in the admin interface is a lazy fetch; so, if you know you’re going to be using a bunch of parts stored in tables, set an eager fetch instead

Here are some areas for code improvement.

  1. My code is pretty clean. It is factored well. I have short methods.
  2. Focus on using Mini Profiler to refactor and reduce network calls. Projections, for instance, can be network heavy.
  3. Don’t uses Ticks as a uniqueId. Rather, increment a counter. Ticks is not unique – don’t do it.
  4. Err on the side of using var – you repeat yourself less.
  5. If I need a transient object (i.e. a bag of properties) to only use in the view, use anonymous objects (or ViewModels) instead of ExpandoObjects.
  6. Create shapes on the fly instead of using @helper methods in Orchard. 
@Display.MyShape(MyProperty: 42, OtherProperty:43)
@{ var shape = New.MyShape(...) }
@Display(shape)