Saturday, January 21, 2017

Applying clean code principles to Deluge script


Robert Martin's seminal book "Clean Code: A Handbook of Agile Software Craftsmanship" lays out a set of best practices for writing code that is easy to understand and therefore easy to maintain. The examples he uses are for Java code but that does not mean it is not applicable to any other language including Deluge script.

Some of the principles I think are particularly valuable are:

1. Use meaningful variable names
I've worked with quite a few subcontractors who use really weird and indecipherable variable names like "a" and "ref" or "B22". It leads to all kind of problems down the line because only the original programmer has any clue what the single letter variable actually means.

I recently encountered an issue where code was written with Excel cell references based on an Excel specification from the client.

One of the On Input scripts looked like this:

E22 = B26 / B5;

This was exactly what was in the Excel specification but the problem was the specification was wrong. The problem would've been immediately apparent if the developer had used proper variable names.

Here's what the code would've looked like had he done that:

annual_revenue = profit_margin_percent * number_of_stores;

Reading that code, you immediately see the problem. That is obviously not how you'd calculate annual revenue. Yet disguised behind Excel cell references, the former code looks passable. You'd only know better if you had the Excel spreadsheet open and thought deeply about what the cell references meant. It's hard to do that and therefore the issue went undetected until we had an embarrassing email from the client asking why the calculations were wrong. It was partly their fault but it was an obvious error that we should've picked up if we'd used proper variable names.

2. Keep functions concise
The other thing that annoys me is when people write custom functions in Deluge that are hundreds of lines long. Don't get me wrong - I've done it in the past and I've experienced the fallout that this creates.

Long functions suck because they:
- are really hard to understand
- take a long time to compile due to Deluge's slow interpretation speed
- are prone to bugs

I really like Martin's suggestion to keep functions short - ideally 5 lines or less. We have custom functions in Zoho Creator so why not use them? One can break up a complex bit of logic into multiple functions.

The same thing goes for HTML pages in Deluge. In one of my projects, we ended up with an HTML page that was over 1000 lines long. Saving the page takes over 60 seconds because Deluge is so slow. We'd have been better off moving the logic into functions rather than inlining it in one big page.

3. Indent properly
This is hard in Zoho Creator because it has an annoying auto-indent feature that never works properly. You end up having to hit backspace a lot to get the indentation right. When you have your code properly indented though it makes a big difference. Well indented code is much easier to understand. Don't believe me? Look through the two code snippets below and tell me which one you prefer?

Poorly indented

crm.createRecord('Leads', params, function (err, data) {
      if (err) {
 console.log('ERROR', JSON.stringify(params), JSON.stringify(err));
        result = {
            successfully_created : false,
        }
      } else {
console.log(JSON.stringify(data))
       


let leadId = ''
        dataResp = data
        dataResp = dataResp.data
        _(dataResp.FL).forEach(function(value) {
   if(value.val == 'Id'){
                leadId = value.content
            }
        });
        result = {
            successfully_created : true,
           

lead_id : leadId
        };
      }

Properly indented
crm.createRecord('Leads', params, function (err, data) {
      if (err) {
        console.log('ERROR', JSON.stringify(params), JSON.stringify(err));
        result = {
            successfully_created : false,
        }
      } else {
        console.log(JSON.stringify(data))
        let leadId = ''
        dataResp = data
        dataResp = dataResp.data
        _(dataResp.FL).forEach(function(value) {
            if(value.val == 'Id'){
                leadId = value.content
            }
        });
        result = {
            successfully_created : true,
            lead_id : leadId
        };
      }

It's the same code both times but the second version is infinitely easier to read.

4. Don't leave commented code in the source
I hate seeing code like this:
input.money_saved_on_retail_admin = input.money_saved_on_retail_admin;
input.online_sales_increased_by_six_percent_value = input.online_sales_increased_by_six_percent_value;
input.cost_of_not_being_able_to_maximise = input.cost_of_not_being_able_to_maximise;
input.Sales_Revenue = input.Sales_Revenue;
input.Gross_Sales_Margin = input.Gross_Sales_Margin;
/*if (input.Send_Mail  &&  (input.Send_Test_Email_to_this_Address  !=  ""))
{
    emp  =  Employee  [ID == input.Sales_Person];
    sendmail
    (
        To       :  input.Send_Test_Email_to_this_Address 
        From     :  zoho.adminuserid 
        CC       :  "mh46v171@gmail.com"
        Subject  :  input.Subject_field 
        Message  :  "Dear Customer,<div><br></div><div>" + input.Email_Body + "<br></div><div><br></div><div>Regards</div><div>" + emp.First_Name + "&nbsp;" + emp.Last_Name + "<br></div><div>Email&nbsp;" + emp.Registered_Email + "</div><div><br></div><div><br></div><div>" + input.ROI_Report + "<br></div><div><br></div><div><br></div>" 
        Attachments : template:ROI_Report as PDF
    )
}
*/
Why is that commented code still there? It serves no purpose other than to confuse the reader and waste their time. Zoho Creator has a versioning system so you can always go back in time to look at the old version of the code. There is no reason to leave commented code in the new version. Just delete it and be done with it.

No comments:

Post a Comment