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.