DRY in views

A designer’s work

We’re working with a designer on PullReview, and we’re very happy about the results so far.. Besides providing us with mock-ups, she also takes care of also the UI (HTML, SCSS, JavaScript). The integration of the initial “static” work into our Rails application was an interesting opportunity to apply DRY principles among the various aspects of the front-end.

An analytics page

Last week, we received the reviewed analytics page. The page shows relevant statistics about the Code Review in a graph looking like this:

1. Initial version

The initial version from the designer is a static one (we’re doing the integration in Rails ourselves). The top part is a canvas drawn using the chart.js library.

<canvas id="chart1" width="330" height="330"></canvas>
  <script>
    var myNewChart = new Chart($("#chart1").get(0).getContext("2d")).Doughnut([
      {
        value: 10, color:"#147488"
      },
      {
        value: 25, color: "#2c8fa4"
      },
      {
        value: 20, color: "#50a8bb"
      },
      {
        value: 10, color: "#7cc1d0"
      },
      {
        value: 35, color: "#00a651"
      }],
      { percentageInnerCutout: 80 });
</script>

As you see, for each part of the graph, both the value and the color need to be given. Also, the script tag is actually inside the HTML, something we’ll need to change at some point.

The bottom part is a simple HTML table skinned using CSS:

 <h5>Review's action category</h5>
   <table>
     <tr>
       <th>Category</th><th>%</th><th>Amount</th><th>Trend</th>
     </tr>
     <tr class="style">
       <td>Style</td><td>10%</td><td>9</td><td><i class="icon-arrow-up"></i></td>
     </tr>
     <tr class="complexity">
       <td>Complexity</td><td>10%</td><td>9</td><td><i class="icon-arrow-up"></i></td>
     </tr>
     <tr class="duplication">
       <td>Duplication</td><td>10%</td><td>9</td><td><i class="icon-arrow-up"></i></td>
     </tr>
     ...
   </table>

2. Make it dynamic

We don’t want to show the same graph to everyone, so the first job was to replace the various values with those coming from our CodeReview model, so that we can get real results. Luckily, We have a nice helper that is able to give us the number of actions for a category for a given CodeReview:

class CodeReviewHelper
  def category_count(code_review, category)
    code_review.violations.count{ |v| v.category == category }
  end
end

We can use it to replace the fixed values in both the JavaScript:

var myNewChart = new Chart($("#chart1").get(0).getContext("2d")).Doughnut([
{
  value: <%= category_count @code_review, :style %>,
  color:"#147488"
},
...

and HTML:

<tr class="style">
  <td>Style</td>
  <td><%= category_count(@code_review, :style) * 100 / @code_review.violations.size %>%</td>
  <td><%= category_count @code_review, :style %></td>
  <td><i class="icon-arrow-up"></i></td>
</tr>
...

3. Put your data into “data-”

Now, having the JavaScript actually use an erb template is not something I find a good idea: it forces the JavaScript to stay in the page (which I don’t want), or to make it a .js.erb, that is, a dynamically generate JavaScript. We want our assets to be static, so they can be cached by the browser, and I prefer my JavaScript to interact only with the DOM.

In order to make this work, I need to have the various values accessible from the DOM itself, and the values in the table are not entirely suited (for instance, I could decide to put units there). Hopefully, the HTML standard now has a set of data attributes specifically for our purpose: to “store custom data private to the page or application”. We can add those to the table rows:

<tr class="style" data-count="<%= category_count(@code_review, :style) %>">
>
  <td>Style</td>
  <td><%= category_count(@code_review, :style) * 100 / @code_review.violations.size %>%</td>
  <td><%= category_count(@code_review, :style) %></td>
  <td><i class="icon-arrow-up"></i></td>
</tr>

The JavaScript code can then retrieve those values directly from the DOM. I’m using jQuery’s handy “.data” method for this:

{
  value: $("tr.style").data("count”)
  color:"#147488"
},

This means that the JavaScript can now be safely exported to its own .js file, as it does no more require any information from the Rails Controller - everything is already in the DOM.

4. Computing CSS using jQuery

This is starting to look good, but I’m still repeating the color: I have each of them once if my CSS file (to color the background of the table rows), and once in the JavaScript (to color the donut parts). Again, jQuery has a nifty function that allows you to get the computed CSS property of any element back. This allows me to rephrase my “color” value: “the same color as the background of the cells”:

{
  value: $("tr.style").data("count")
  color: $("tr.style td").css("background-color")
},

Looking even better.

5. Loops and controller

Now, I still have a rather large table with a lot of repetition:

<tr class="style">
  <td>Style</td>
  <td><%= category_count(@code_review, :style) * 100 / @code_review.violations.size %>%</td>
  <td><%= category_count @code_review, :style %></td>
  <td><i class="icon-arrow-up"></i></td>
</tr>
<tr class="documentation">
  <td>documentation</td>
  <td><%= category_count(@code_review, :documentation) * 100 / @code_review.violations.size %>%</td>
  <td><%= category_count @code_review, :documentation %></td>
  <td><i class="icon-arrow-up"></i></td>
</tr>
<tr class="duplication">
  <td>duplication</td>
  <td><%= category_count(@code_review, :duplication) * 100 / @code_review.violations.size %>%</td>
  <td><%= category_count @code_review, :duplication %></td>
  <td><i class="icon-arrow-up"></i></td>
</tr>

This could be replaced with a standard loop, and the categories count moved to the controller, so that we just have to iterate on an array in the view:

<% @categories.each do |category| %>
  <tr class="<%= category[:code] %> value" data-count="<%= category[:value] %>">
    <td><%= category[:code].to_s %></td>
    <td><%= category[:value] * 100 / category[:total] %>%</td>
    <td><%= category[:value] %></td>
    <td><i class="icon-arrow-up"></i></td>
  </tr>
<% end %>

The controller can take charge of creating the small Hashes:

class CodeReviewsController < ApplicationController
  def analytics
  ...
  @categories = [:style, :documentation, :duplication].map do |category_code|
      {
        code: category_code,
        value: category_count(@code_review, category_code),
        total: @code_review.violations_to_check.size
      }
    end
  end
...
end

This requires the helper method to be moved into the controller. My problem is that I’m still using it in the view too. Of course, I don’t want it duplicated (even if it is a one line method). Rails has a nice solution. You can define any method in a controller has being actually a “helper method” (hence accessible in the view):

class CodeReviewsController < ApplicationController
  helper_method :category_count, :severity_count

6. Convert to CoffeeScript

Finally, as we tend to use CoffeeScript over JavaScript, I converted the small JavaScript to Coffee. Although I like it, I’m still more used to JavaScript. Fortunately, there is a tool that makes the reverse compilation, converting JavaScript to CoffeeScript: js2coffee.org. I also took the opportunity to wrap my code in a dedicated function.

Result

The general result is quite nice with a single short coffeescript, and a 10 lines erb file, both using the SCSS for styling:

analytics.js.coffee

drawGraph = (baseElement) ->
  values = $.map($(baseElement).find("table").find("tr.value"), (val, index) ->
    value: $(val).data("count")
    color: $(val).children("td").css("background-color")
  )
  myNewChart = new Chart($(baseElement).find("canvas").get(0).getContext("2d")).Doughnut(values,
    percentageInnerCutout: 80
  )

analytics.html.erb

<canvas width="330" height="330"></canvas>
    <table>
      <tr>
        <th>Category</th><th>%</th><th>Amount</th><th>Trend</th>
      </tr>

      <% @categories.each do |category| %>
        <tr class="<%= category[:code] %> value" data-count="<%= category[:value] %>">
        <td><%= category[:value] %></td><td><%= category[:value] * 100 / category[:total] %>%</td><td><%= category[:value] %></td><td><i class="icon-arrow-up"></i></td>
      </tr>
      <% end %>
    </table>

Conclusion

This was a good opportunity for me to apply – to the Front End – the principles I believe in and apply most often to the backend: DRY, and good separation of concerns. Let the controller compute the values, the view show the static part, the JavaScript draw on a canvas, and the SCSS be the unique place for all styling. Ruby, Rails, SCSS and jQuery help to make this both painless and enjoyable.

On a totally different note, this exercise did teach us another important lesson: we really want our designer to be able to work immediately on our codebase.

Back soon, with new learnings while developing PullReview! If you can’t wait, you can subscribe here.

 

Enhanced by Zemanta




10 thoughts on “DRY in views

  1. Alexander Zaytsev

    Good read. I’m using a similar technique with my javascript code, only that I put it into a “class”, not a function (it gives you more flexibility).

    Another thing, I don’t agree on the step number 2. Why did you put this code into a view helper? I think it belongs in a model.

    Reply
    1. Martin Post author

      Hi Alexander,
      I’m not that used to classes” in JavaScript – JS objects always looks to me as Hashes. Should the code become more complicated, grouping the functions in objects would be a good idea, but the example here was for me simple enough to keep in a single (simple) function.

      Regarding the model vs helper, the line is often fine. We have multiple representation of a CodeReview (a rather complex object in itself), so we try not to push too much on the object itself (less it become really bloated), keeping the model really about the business, and exporting representation problems into controllers. A middle approach would be do define a CodeReviewPresenter, that would wrap/decorate the CodeReview and define the varous methods useful for the front-end.

      Thanks for your input!

      Martin

      Reply
    2. Scott

      I don’t agree with number 2 either. I think this whole thing could even be simplified if you were using the correct query.


      @code_review = CodeReview.find(params[:id])
      @categories = @code_review.violations.group('category').count

      Which would produce a hash of category => count

      {"style" => #, "documentation" => #, ....}

      I would use a helper method for the percentage, which rails has already.

      Reply
      1. Scott

        That is, when you you do the percentage in the view you could just do

        <tr class=" value" data-count="">
        %

        And I would make sure that the @code_review.violations_to_check.size is not making duplicate queries each time it is requested. If it is then you might want to assign that to an instance variable as well.

        Reply
      2. Martin Post author

        Hi Scott,
        Nice tip for the Rails percentage – did not know about that one. The real page is a bit more complex (we actually group along several different axes, and are showing different graphs), but I agree that generating them in a single swoop in the controller is both more efficient and more elegant.

        Thanks for your remark,

        Martin

        Reply
        1. Scott

          I guess to fully call your view DRY it might be best to implement a Presenter like you mentioned earlier. Maybe you could write a post about doing that in order to DRY up your view since you seem to have multiple components on the page which you don’t really show in this post, I think that refactor could be valuable.

          I also want to point out that your implementation has a lot of loops.
          - One for creating the categories map in the controller.
          - One nested inside to find the violation count for a specific category in the helper method.
          - And then one for looping over the categories map again in the view.

          The nested one is a little concerning. If I were to keep this implementation I would define a scope on the Violation model to find the violations by category.

          In Violation

          scope :by_category, lambda { |c| where(category: c) }

          Then back to your helper

          code_review.violations.by_category(category).count

          That would get rid of that nested loop using the array count (query + array count) and replace it with a query that returns the count.

          Reply
          1. Martin Post author

            Hi Scott,
            Good idea about the follow-up, and it will push me to get it better in my codebase too, which is even better. The Presenter way looks definitively good here as the object is quite complex.

            Good observation about the loops too – guess I have to look again to it.

            Thanks,

            Martin

  2. jayden

    An appealing discussion will be worth comment. I do believe that you should create more on this kind of topic, it might not be a taboo subject matter but normally people are insufficient to speak upon such topics. To the next. Cheers

    Reply

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>