Validations in rails, improved

Before

<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><div style="text-align: left;"> </div><p style="text-align: left;"><strong>After</strong></p><div style="text-align: left;"> </div><table style="text-align: left; margin-left: 0px; margin-right: auto;" class="CodeRay"><tbody><tr> <td title="click to toggle" class="line_numbers"><br /></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.