mc9
mc9

Reputation: 6341

Ruby instance variable changes unexpectedly

In the code below, a cluster has many points.

class Cluster
  attr_accessor :centroid, :points
  def initialize(centroid, *points)
    @centroid = centroid
    @points = points
  end
end

class Point
  attr_accessor :x, :y
  def initialize(x = 0, y = 0)
    @x = x
    @y = y
  end
end

An example of a Cluster object (lets us call this c):

#<Cluster:0x007ff5c123c210
 @centroid=#<Point:0x007ff5c123c288 @x=25, @y=125>,
 @points=
  [#<Point:0x007ff5c123c238 @x=25, @y=125>,
   #<Point:0x007ff5c1020120 @x=28, @y=145>]>

I am trying to calculate the mean of the points, and update @centroid without changing @points.

Let's say I have:

class Point
  def +(point)
    @x = @x + point.x
    @y = @y + point.y
    self
  end
  def /(num)
    @x = @x/num
    @y = @y/num
    self
  end
end

and to calculate the mean of all the points, I run:

c.centroid = c.points.reduce(&:+)/c.points.length

Then, c changes to:

#<Cluster:0x007ff5c123c210
 @centroid=#<Point:0x007ff5c1515ec8 @x=26, @y=135>,
 @points=
  [#<Point:0x007ff5c1515ec8 @x=26, @y=135>,
   #<Point:0x007ff5c1020120 @x=28, @y=145>]>

Note that first element of the @points is changed. Any suggestions?

Upvotes: 2

Views: 62

Answers (3)

Chowlett
Chowlett

Reputation: 46675

Your + method in Point modifies the point's members @x and @y. You need to return a new point with the calculated values instead:

def +(point)
  Point.new(@x + point.x, @y + point.y)
end

def /(num)
  Point.new(@x/num, @y/num)
end

Upvotes: 5

sawa
sawa

Reputation: 168249

Since you did not pass an initial value to reduce, the first point became modified. You can pass a new point as the initial value to reduce, which will be modified and returned.

c.centroid = c.points.reduce(Point.new, &:+)/c.points.length

Upvotes: 2

ReggieB
ReggieB

Reputation: 8257

I think the problem is caused by the + method modifying the point @x and @y values.

Try changing the + method to:

def +(point)
  x = @x + point.x
  y = @y + point.y

  self.class.new(x, y)
end

Upvotes: 0

Related Questions