“Validations in rails improvised”

This refactoring is based on a topic Marcel and I covered at RailsConf Europe.

Before

1<tt><br /></tt>2<tt><br /></tt>3<tt><br /></tt>4<tt><br /></tt><strong>5</strong><tt><br /></tt>6<tt><br /></tt>7<tt><br /></tt>8<tt><br /></tt>9<tt><br /></tt><strong>10</strong><tt><br /></tt>11<tt><br /></tt>12<tt><br /></tt>13<tt> </tt></pre></td> <td class="code"><pre><tt><br /></tt><span class="r">class</span> <span class="cl">Expense</span> < <span class="co">ActiveRecord</span>::<span class="co">Base</span><tt><br /></tt> belongs_to <span class="sy">:payee</span><tt><br /></tt> protected<tt><br /></tt> <tt><br /></tt> <span class="c"># Nice and concise, but what happens as we add more rules</span><tt><br /></tt> <span class="c"># and how do we write test cases for the four different possible </span><tt><br /></tt> <span class="c"># validation states?</span><tt><br /></tt> <span class="r">def</span> <span class="fu">validate</span><tt><br /></tt> errors.add(<span class="s"><span class="dl">"</span><span class="k">Not enough funds</span><span class="dl">"</span></span>) <span class="r">if</span> payee.balance - amount > <span class="i">0</span><tt><br /></tt> errors.add(<span class="s"><span class="dl">"</span><span class="k">Charge is too great</span><span class="dl">"</span></span>) <span class="r">if</span> payee.account.maximum_allowable_charge > amount<tt><br /></tt> <span class="r">end</span><tt><br /></tt><span class="r">end</span><tt> </tt></pre></td> </tr></tbody></table> <p><strong>After</strong></p> <table class="CodeRay"><tbody><tr> <td title="click to toggle" class="line_numbers"><pre>1<tt><br /></tt>2<tt><br /></tt>3<tt><br /></tt>4<tt><br /></tt><strong>5</strong><tt><br /></tt>6<tt><br /></tt>7<tt><br /></tt>8<tt><br /></tt>9<tt><br /></tt><strong>10</strong><tt><br /></tt>11<tt><br /></tt>12<tt><br /></tt>13<tt><br /></tt>14<tt><br /></tt><strong>15</strong><tt><br /></tt>16<tt><br /></tt>17<tt><br /></tt>18<tt><br /></tt>19<tt><br /></tt><strong>20</strong><tt><br /></tt>21<tt><br /></tt>22<tt><br /></tt>23<tt><br /></tt>24<tt><br /></tt><strong>25</strong><tt><br /></tt>26<tt><br /></tt>27<tt><br /></tt>28<tt><br /></tt>29<tt><br /></tt><strong>30</strong><tt><br /></tt>31<tt><br /></tt>32<tt><br /></tt>33<tt><br /></tt>34<tt><br /></tt><strong>35</strong><tt><br /></tt>36<tt><br /></tt>37<tt> </tt></pre></td> <td class="code"><pre><tt><br /></tt><span class="r">class</span> <span class="cl">Expense</span> < <span class="co">ActiveRecord</span>::<span class="co">Base</span><tt><br /></tt> belongs_to <span class="sy">:payee</span><tt><br /></tt> <tt><br /></tt> <span class="c"># Instead of one large validation method, break each individual</span><tt><br /></tt> <span class="c"># rule into methods, and declare them here.</span><tt><br /></tt> validate <span class="sy">:ensure_balance_is_sufficient_to_cover_amount</span><tt><br /></tt> validate <span class="sy">:amount_does_not_exceed_maximum_allowable_charge</span><tt><br /></tt><tt><br /></tt> protected<tt><br /></tt><tt><br /></tt> <span class="c"># These validation callbacks simply add error messages if a particular </span><tt><br /></tt> <span class="c"># condition is met. Each of them can be tested and understood on their own</span><tt><br /></tt> <span class="c"># without having to understand the entire body of the validate method.</span><tt><br /></tt> <span class="r">def</span> <span class="fu">ensure_balance_is_sufficient_to_cover_amount</span><tt><br /></tt> errors.add(<span class="s"><span class="dl">"</span><span class="k">Not enough funds</span><span class="dl">"</span></span>) <span class="r">if</span> insufficient_funds?<tt><br /></tt> <span class="r">end</span><tt><br /></tt> <tt><br /></tt> <span class="r">def</span> <span class="fu">amount_does_not_exceed_maximum_allowable_charge</span><tt><br /></tt> errors.add(<span class="s"><span class="dl">"</span><span class="k">Charge is too great</span><span class="dl">"</span></span>) <span class="r">if</span> exceeds_maximum_allowable_charge?<tt><br /></tt> <span class="r">end</span><tt><br /></tt> <tt><br /></tt> <span class="c"># By defining separate predicate methods we can test each of them individually</span><tt><br /></tt> <span class="c"># and new programmers can see the intent of the code, not just the implementation.</span><tt><br /></tt><tt><br /></tt><tt><br /></tt> <span class="c"># Instead of subtracting the amount from the balance and checking if the</span><tt><br /></tt> <span class="c"># value is greater than 0, change the implementation to mirror the intent.</span><tt><br /></tt> <span class="c"># There was a bug in the before code, this is more obvious.</span><tt><br /></tt> <span class="r">def</span> <span class="fu">insufficient_funds?</span><tt><br /></tt> amount > payee.balance<tt><br /></tt> <span class="r">end</span><tt><br /></tt> <tt><br /></tt> <span class="r">def</span> <span class="fu">exceeds_maximum_allowable_charge?</span><tt><br /></tt> payee.account.maximum_allowable_charge > amount<tt><br /></tt> <span class="r">end</span><tt><br /></tt><span class="r">end</span><tt> </tt>

While the refactored version may have
more lines of code, but don’t let that scare you. It’s far more
important for code to be human readable than incredibly concise.