Refactoring MonoRail Views
As I mentioned, I build a very quick & dirty solution to display a collection of scheduled task descriptions. The end result looked like this:
This works, and it should start producing value as of next week, but I didn't really like the way I built it.
Here is the original view code:
<table cellspacing="0" cellpadding="0"> <thead> <tr> <th> Name:</th> <th> Occurances</th> <th> </th> </tr> </thead> <tbody> <% for task in tasks: %> <tr> <td> ${Text.PascalCaseToWord(task.Name)} </td> <td> Every ${task.OccuranceEvery} </td> <td> ${Html.LinkTo("Execute", "ScheduledTasks", "Execute", task.FullName)} </td> </tr> <% end %> </tbody> </table>
This work, it is simple and easy to understand, but it still bothered me. So I replaced it with this:
<% component SmartGrid, {@source: tasks} %>
Well, that was much shorter, but the result was this...
I am cropping things, because it is a fairly long picture, but it should be clear that this is not a really nice UI to use.
This was my second attempt;
<% component SmartGrid, {@source: tasks, @columns: [@Name, @OccuranceEvery] } %>
And it produced this:
Better, but not really that much, let us try to have nicer names there, shall we?
<% component SmartGrid, {@source: tasks, @columns: [@Name, @OccuranceEvery] }: section Name: %> <td>${Text.PascalCaseToWord(value)}</td> <% end end %>
And this produced:
That is much better on the name side, but we still have the "Occurance Every" column to fix...
<% component SmartGrid, {@source: tasks, @columns: [@Name, @OccuranceEvery] }: section OccuranceEveryHeader: %> <th>Occurances</th> <% end section Name: %> <td>${Text.PascalCaseToWord(value)}</td> <% end section OccuranceEvery: %> <td>Every ${value}</td> <% end end %>
With the result being:
One last thing that we have left is the additional column at the end, we can manage it like this:
<% component SmartGrid, {@source: tasks, @columns: [@Name, @OccuranceEvery] }: section OccuranceEveryHeader: %> <th>Occurances</th> <% end section MoreHeader: %> <th></th> <% end section Name: %> <td>${Text.PascalCaseToWord(value)}</td> <% end section OccuranceEvery: %> <td>Every ${value}</td> <% end section More: %> <td>${Html.LinkTo("Execute", "ScheduledTasks", "Execute", item.FullName)}</td> <% end end %>
So here is the final result:
Now that I did that, I am looking at both pieces of code and wondering:
- What is the fuss about, anyway?
- Which of those versions is more readable?
Granted, this is a fairly specialized case, but in terms of LoC, the second approach is actually longer, and the "major" benefit here is that I get less HTML in the view, but that is not a really major consideration.
The SmartGrid would produce a pager if needed, but that about it with regards to the differences in their abilities.
Comments
Oh, I thought the refactoring was just going to be fixing the spelling of "occurrance" -> "occurrence".
Erm, I'd go for the first way every time. Like you said, the second approach is longer, it's much less readable, and the benefit of having less HTML in the view is hardly a benefit. The view is HTML. That's what it's there for. The second approach smells much of ASP.NET, and not much of MonoRail. If you wanted to have a HTML/CSS/non-programmar work on your views, I'm sure she'd look at your second approach and say wtf, but the first approach is much more soluble.
Keep simple things simple... I totally agree with Aaron here.
Comment preview