Custom delete method refactoring

Posted on

Problem

I’m new to Ruby and don’t know all the methods, so I was thinking there would probably be and easier way to do something like this. I’d love any input on how to refactor this.

   define_singleton_method(:delete) do |id|
        revised_words = []
        @@words.each do |word|
          if word.id() != id
            revised_words.push(word)
          else
            Definition.delete_by_word_id(id)
          end
        end
        @@words = revised_words
      end

Solution

Some notes:

  • An array is not the best data structure to store the words, operations will be O(n). Better a hash {word_id => word}

  • Use 2-space indentation.

Then you can write:

define_singleton_method(:delete) do |id|
  if @@words.delete(id)        
    Definition.delete_by_word_id(id)
  end
end

The method would benefit from functional programming:

@@words.select{|word| word.id == id}.each{|id| Definition.delete_word_by_id(id)}
@@words.select{|word| word.id != id}

Still I don’t like that this method both has side effects and returns, it has too much responsability.

Leave a Reply

Your email address will not be published. Required fields are marked *