Ruby class variable behaving in unexpected way

Question

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}

Could you provide a minimum working example so we can actually see what's going on? Also, double-check that your eql? (I think that's what's used) is implemented correctly. — anon
– anon, Commented May 27, 2015 at 20:17
How does bad work? Could it be that they become bad between the call to delete_bad_widgets and the next call to all? — msergeant
– msergeant, Commented May 27, 2015 at 20:21
I just created a minimalist version of what you have and it worked fine when i called the delete function manually. Make sure your widget.bad conditional is working as you think it is. — MCBama
– MCBama, Commented May 27, 2015 at 20:25

knut · Accepted Answer · 2015-05-27 20:37:49Z

3

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.

answered May 27, 2015 at 20:37

knut

28k6 gold badges88 silver badges113 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

erdostom Over a year ago

Thank you, I accepted the answer below, but your answer really helped me understand what's going on!

neuronaut · Accepted Answer · 2015-05-27 20:47:14Z

2

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).

edited May 27, 2015 at 20:47

answered May 27, 2015 at 20:39

neuronaut

2,71919 silver badges24 bronze badges

Collectives™ on Stack Overflow

Ruby class variable behaving in unexpected way

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related