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.

The last lecture by Randy Pausch

This book is part of my new years resolution . It is second in the series.

It is not a fiction novel. It is as true as it gets out there. It will change the way you look at life and if it will not bring out such radical changes in your life, it will at least make you re-think of what you have been doing with your life until now. And if this does not bring any of these things to your mind, I do not know what will.

It is the story of Randy Pausch who was a professor at Carnegie Mellon University. He was also many other things. In 2006, he was diagnosed with pancreatic cancer. And he died in 2008. But he did gave a ‘last lecture’ at Carnegie Mellon. And boy that lecture was something. And he said that this lecture was the last fake-head he gave. Go ahead and find out what the term fake-head means by reading the book. I am not going to spoil it for you. This lecture was not meant for the audience inside the auditorium but for his three kids who were so young at that time to really understand or remember any of this. So, he wanted to leave something for his kids to look upto to find out about their dad. And he also said if he was a painter, he would paint something for them. And since he was a professor, so he lectured for them.

Now I am not going to give out any more details about the book. But I am going to tell you my favourite lines from the book or stories from it as this book is full of amazing quotes and awesome stories. Here goes:

1.

Brick walls are there for a reason, it gives us a chance to show how badly we want something

Brick walls here are the obstacles you face when you want to achieve something. And if you really want to do it, you will find a way to do it.

2.

Time is all you have. And one day you may find that you have less than what you think.

This one he learnt the hard way when he was diagnosed with pancreatic cancer and was given a 3-6 months of healthy life. And I have read it time and time again, people who suffer from such diseases or somebody who have near death experience, they all have same thing to see. That life is precious, do not cry about petty things in life and do what you gotta do.

3.

Experience is what you get when you do not get what you wanted

This is a great advice. We all get disappointed when we fail at something after we have worked so hard to achieve it. But I will remember this one that I have just found out a way to fail at something and I am not going to repeat the same mistakes.
I read this in an article about entrepreneurs. It said that companies are more inclined to hire people who have failed at something, specially at the level of CEO’s as they are lot less likely to fail than someone who has never tasted failure.

4. On when people complain about government or an organisation spend billions of dollars to achieve something like trying to land people on the moon. When that money could be used to serve the hungry and try and solve the hunger problems in the country and the world.

When you use money to fight poverty, you are working at the margins. But when you’re spending billions to put people on the moon, you are inspiring all of us.

True indeed ..!!

5.

Complaining does not work as a strategy. We all have finite time and energy. Any time we spend whining is unlikely to help us achieve our goals. And it won’t make us happier.

6. And a story for the last.
Randy said this is a lesson for the folks at management.
When Randy and his sister( who is 2 years older than him ) were kids. They went to Disneyland. And they went to one of the gift shops. And they bought a salt and pepper bottle from the shop. And when they went out of the shop, it fell from their hands, hit the ground and broke.
A man who saw this happen told Randy to go back to the shop and ask them to replace the gift. And Randy said why would they do that. It is not their fault. I broke it. I should have been careful.
Then the man said, there is no harm in trying. So the kids went back to the shop and told the guys working at the shop about how exactly it broke. And the shopkeeper replaced the broken one with a new one and did not ask them to pay for it again. And he also said that it was probably our mistake that we did not package it good enough to not brake when it accidentally fells on the ground. So the kids were obviously happy. They went back home and told this story to their parents and they always remembered this little gesture from the shopkeeper. And over the years it was a tradition sort of to visit Disneyland and Randy’s parents spent more than 100,000$ in tickets and souvenirs. So all in all it was good investment of $10 by the Disney shopkeeper which evaluated to $100K over the years.

I think this is crucial to every one as it not only tells us how we should treat our customers, but also tells us how we should treat each other. How there should be humility, forgiveness and willingness to help each other and it is only then we can help to make this world a better place to live.

I hope you will read the book and get inspired. And this is the youtube link to the video of the last lecture by Randy Pausch.