Using Null Object Pattern in Rails to improve code quality

If you have not read my previous post I wrote about Service classes, I would suggest to read that first as it is in continuation to the previous post. This is the link to previous post.

In this post we are going to look at another pattern that helps us in refactoring code, especially helps us in removing conditions. It is called Null Object Pattern.

This code is used to download the data dump based on some rules as always. But here is my 2 cents, do not even try to understand the code.

   def download_measurements
    specification = @inspection.specification
    serial_numbers = @inspection.get_serial_numbers
    inspection_results = @inspection.inspection_results.includes(:inspection_serial_number)
    row_list = []

    row_list << ['unit number', 'serial number', 'gage id', 
                 'inspected by', 'inspected on yyyy-mm-dd', 
                 'place', 'measurement', 'bonus tolerance', 
                 'actual measurement', 'text', 'remark', 
                 'non-conformance number']

    serial_numbers.each_with_index do |serial_number, index|
      specification.feature_repeats.times do |place_index|
        place_number = place_index + 1
        ir = inspection_results.collect{|i| 
             i if i.serial_number.number == serial_number}.
             uniq.compact.first

        specification.readings_required.times do |measurement_index|
          measurement_number = measurement_index + 1
          rm = ir.nil? ? nil : 
            ir.result_measurements.where(:place_number => place_number, 
            :measurement_number => measurement_number).first
          
          inspected_on_date = ir.nil? ? nil : ir.inspected_on.nil? ?
          nil : ir.inspected_on.strftime('%Y-%m-%d') 
          
          measurement_value = rm.try(:actual_measurement).nil? ? 
          nil : rm.try(:actual_measurement)
         
          measurement_value = rm.try(:measurement_result) == 'P' ? 'PASS' : rm.try(:measurement_result) == 'F' ? 'FAIL' : nil if specification.result_type == 'Attr'
          row_list << [index+1, serial_number, ir.try(:gage_serial_number), 
                       ir.try(:inspected_by), inspected_on_date, place_number, 
                       measurement_number, nil, measurement_value, rm.try(:information),
                       ir.try(:remarks), ir.try(:non_conformance_number)]
        end
      end
    end
    file_name = "Template" + "-" + @inspection.balloon + "_#{Time.now.to_i}.xlsx"
    file_path = "#{Rails.root}/tmp/#{file_name}"
    XlsxHelper.generate(row_list, file_path)
     send_file(File.open(file_path), :type => 'application/vnd.ms-excel', :filename => file_name)
   end

We clearly need to refactor because this code is difficult to understand even for the person who has written the code, forget the team who is working on the same project. And since this code is difficult to understand, it is difficult to debug and it is difficult to change as changes will come as they always do.

As I can see that this whole code first of all should not be a part of a controller method. Since the purpose of this code is to generate a sort of data dump in a csv file, I would make it a service class and call the methods of that class as explained in the previous post.

By just glancing at the code, I can understand that there is a header added somewhere in there and then there is some logic to add the data. So I will start by making a service class. And make these empty methods.

class ExportIdeData
  def initialize(inspection)
    @inspection = inspection
  end

  def run!
  end

  private

  def header
  end

  def add_data
  end
end

And my aim is to make controller code look like this:


  def download_measurements
    ide_data_runner = ExportIdeData.new(@inspection)
    ide_data_runner.run!
    generate_file(ide_data_runner)
  end

Now that we have defined a service class and has added methods to it, lets focus on this line of the old code:

  inspected_on_date = ir.nil? ? nil : ir.inspected_on.nil? 
    ? nil : ir.inspected_on.strftime('%Y-%m-%d') 

In this code, we see a complex logic of conditions which decide what value to return. If ir object does not exist, we want the value nil, if ir object exists but no value for attribute inspected_on exists in db, then we want nil. And if the value exists in db, we want to change the format of the date and then return that value.

If I have to apply such conditions to lets say only 1 attribute of the model, I wont mind adding all the conditions for once. But in our use case, when we have to apply such logic to multiple attributes of a model, it won’t just make sense to compromise on the code quality. And thus, Null Object Pattern helps us in cleaning our code.

So these are some of the changes that we make to our new service class.

  def header
    @rows = [['unit number', 'serial number', 'gage id',
              'inspected by', 'inspected on yyyy-mm-dd',
              'place',  'measurement', 'bonus tolerance',
              'actual measurement', 'text', 'remark', 
              'non-conformance number']]
  end

  def add_data(sheet)
    @serial_numbers.each_with_index do |sn, index|
      @place_measurements.each do |pc|
        ir = find_inspection_result(sn)
        rm = find_result_measurement(ir, pc[0], pc[1])
        data = [index+1, sn.number, 
                ir.serial_number, ir.inspected_by, 
                ir.inspected_on_rep, pc[0], pc[1], nil, 
                rm.measurement_value, ir.remarks, 
                ir.non_conformance_number]

        add_row_with(sheet, data, @header_values, 
                     height=27.571428571428573)
      end
    end
  end

Now do not get too much into the code. Just have a quick look at it, this code looks clean.The whole code is not important for you to understand for what we are trying to achieve here, pay attention to these lines:

  ir = find_inspection_result(sn)
  rm = find_result_measurement(ir, pc[0], pc[1])

These are some of the methods that that we need to define:

  def find_inspection_result(serial_number)
    ir = @inspection.results.where(:serial_number_id => serial_number.id).first
         || NilInspectionResult.new
    ir
  end

In the above code, I find the inspection result object and if it fails to find such object in database, then we initialize a new object of the class NilInspectionResult.
Keep in mind these are classes that do have a corresponding database table. It is just a plain ruby class which happen to be created under models folder.

This is how it looks:

class NilInspectionResult
    
  def gage_serial_number
    nil
  end
    
  def inspected_by
    nil
  end 
    
  def inspected_on_rep
    nil
  end
  
  def remarks
    nil
  end
              
  def non_conformance_number
    nil
  end 
end 

So, when we do not find an result object in database, a new object of our newly defined class NilInspectionResult has access to these methods defined above which a valid result object would have.
What I mean here is that remarks, non_conformance_number etc are attributes or methods defined in InspectionResult model and so we define the same methods in our new class also with values we expect them to return.

This also helps us in keeping our code clean but also easy to change. For ex: In future if client wants that instead of returning nil as the date for the case when result object does not exist, return the current date. So I just have to change the method in NilInspectionResult class to return current date and not nil.

Similarly look at this code:

measurement_value = rm.try(:actual_measurement).nil? 
  ? nil : rm.try(:actual_measurement)

‘try’ is just a fancy way to write if else conditions. And it is suggested to avoid such conditions whenever possible. So I would change the code to something like this:

def find_result_measurement(ir, pn, pm)
  rm = ir.result_measurements.where(:place_number => pn, :measurement_number => mn).first 
       rescue NilResultMeasurement.new
  rm
end

Here we find the result measurement object which also depends on the ir object. And in our previous method we have found the result object ir, and it could contain the actual inspection result object or a nil template of it(NilInspectionResult.new).

So in code to find the result measurement object, I use rescue to initialize a NilResultMeasurement object. And I add a class like this:

class NilResultMeasurement
  def measurement_value
    nil 
  end

  def pass_fail_result
    'N/A'
  end 
end

Now I think I do not need to explain why this above code is what it is. But one of the most important thing to remember when using Null Object Pattern is not to overuse it.

So I hope this helps you in avoiding conditional blocks whenever and wherever feasible to use this pattern. And I will say this again, we have not reduced the number of lines of code, we have infact increased the number of lines of code. But we have made our code looks much cleaner, easier to change and understand.

Advertisements

4 thoughts on “Using Null Object Pattern in Rails to improve code quality

  1. You don’t need to return nil explicitly:

    class NilInspectionResult
    def gage_serial_number; end
    def inspected_by; end
    def inspected_on_rep; end
    def remarks; end
    def non_conformance_number; end
    end

    Or we can use attr_reader as a shorthand:

    class NilInspectionResult
    attr_reader :gage_serial_number,
    :inspected_by,
    :inspected_on_rep,
    :remarks,
    :non_conformance_number
    end

    Structs seems to be best for creating null objets:

    NilInspectionResult = Struct.new(
    :gage_serial_number,
    :inspected_by,
    :inspected_on_rep,
    :remarks,
    :non_conformance_number
    end

    1. Yes, this is indeed correct and a more concise way of doing this. I did this to make it really explicit for clear understanding.

      ThankYou for the gist.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s