CODEWITHSUNDEEP
ruby
erdostom
erdostom

Reputation: 387

Ruby class variable behaving in unexpected way

I have a class variable that collects all the instance variables. It also had a method for getting them and deleting them.

module Somemodule
  class Widget
    @@widgets = []
    def initialize
      ...
      @@widgets << self
    end
    def self.all
      @@widgets
    end
    def delete
      @@widgets.delete(self)
    end
  end
end

In my code, I want to delete widgets sometimes. 

module Somemodule
  def self.delete_bad_widgets
    Widget.all.each do |widget|
      if widget.bad
        widget.delete
      end
    end
  end
end

The problem is that this doesn't delete all the bad widgets. If I call Widget.all afterwards, I can still see bad widgets. Anyone have any idea what's going on? I'm running Ruby 2.2.1.

Thank you

EDIT: all in all there are about 4500 widgets and the code changes them a couple of times. But I can still run something like the following (in the same delete_bad_widgets method!) and still see bad widgets:

Widget.all.each {|w| pp w if w.bad}

Upvotes: 1

Views: 75

Answers (2)

neuronaut
neuronaut

Reputation: 2709

Try using reject! to remove the bad widgets instead of doing the iteration yourself.

module Somemodule
  def self.delete_bad_widgets
    Widget.all.reject! {|widget| widget.bad}
  end
end

As knut's answer points out, using delete while iterating over an array will cause some elements to be skipped. It's generally not a good idea to insert/delete elements of a collection at the same time you are iterating over that collection.

Also, using reject! (a.k.a. delete_if) will perform better since delete will essentially iterate through the entire array with each call (i.e. reject! give O(n) performance where -- the way you've used it -- delete will give O(n^2) worst case performance).

Upvotes: 2

knut
knut

Reputation: 27875

If you delete entries during an each it influences the loop.

Just see this example:

a = [1,2,3,4,5,6,7]

p a
a.each{|x|
  p x
  a.delete(x) if x == 3
}
p a

The result:

  [1, 2, 3, 4, 5, 6, 7]
  1
  2
  3
  5
  6
  7
  [1, 2, 4, 5, 6, 7]

The entry 4 is not printed! The each-loop keeps the index, but the sequences changed with your deletion.

I would recommend to implement a class method delete_bad or a delete_if.

Upvotes: 3

Related Questions