Design tip: Watch how you cut!

The number one design problem I see people do is writing code that is overspecific. By that I mean code that can only be used in one place and can never be reused. Programmers should always try to write code that is generic, reusable and independent from the context where it is used.

Design is (mainly) about breaking the solution in small, manageable pieces. However, how you do the cut is important; if you cut poorly, you end up with a worse mess than when you had a single monolithic piece.

This is an example I saw last week. It is Rails-specific but I think non-Rails readers will be able to follow the logic. The application had a general HTML layout that contained something like this:

<table>
  <tr>
    <!-- main application content -->
    <td valign="top"><%= yield %></td>

    <!-- right sidebar -->
    <%= yield :right_sidebar %>
  </tr>
</table>

Ignore for a second the use of a table for layout (that requires a separate post!) Can you see the design flaw?

Yes, the flaw is that all pages that want to add something to the right sidebar must remember to wrap it in <td valign="top">...</td>. Like this:

<% content_for :right_sidebar do %>
<td valign="top">
  BANNER GOES HERE
</td>
<% end %>

In practice the code that goes in the right sidebar is dependent on the context where it is used. The solution in this case is simple: keep the context in the calling code, not in the called code. Like this:

<table>
  <tr>
    <!-- main application content -->
    <td valign="top"><%= yield %></td>

    <!-- right sidebar -->
    <td valign="top">
      <%= yield :right_sidebar %>
    </td>
  </tr>
</table>

This has the problem that if the page does not want to put anything in the right banner, it will still have an empty td element. Fortunately, Rails has a clean solution for this:

<table>
  <tr>
    <!-- main application content -->
    <td valign="top"><%= yield %></td>

    <!-- right sidebar -->
    <% if content_for? :right_sidebar %>
    <td valign="top">
      <%= yield :right_sidebar %>
    </td>
    <% end %>
  </tr>
</table>

The code that uses the right sidebar is now simplified to

<% content_for :right_sidebar do %>
  BANNER GOES HERE
<% end %>

And this code will not have to change when the application layout is changed to something that does not use tables.

Watch how you cut!

Leave a Reply