2f6686bd2c6bb633a1cbf7b5a73c8491

So here here my ugly code. Should be nice if ruby metaprogramming was used here.

object.constraints.each do |c|
  p = object.particles[c.index]
  if c.component == :x
    if c.dir == :inf
      p.x = c.value if p.x < c.value
    else
      p.x = c.value if p.x > c.value
    end
  elsif c.component == :y
    if c.dir == :inf
      p.y = c.value if p.y < c.value
    else
      p.y = c.value if p.y > c.value
    end
  elsif c.component == :z
    if c.dir == :inf
      p.z = c.value if p.z < c.value
    else
      p.z = c.value if p.z > c.value
    end
  end
end

Refactorings

No refactoring yet !

880cbab435f00197613c9cc2065b4f5a

danielharan

January 6, 2009, January 06, 2009 01:47, permalink

1 rating. Login to rate!

c.component sounds like a type. Assuming it can only be equal to :x, :y or :z you could do:

class SomeObjectWithComponent
  def component_value=(value)
    send("#{component}=", value)
  end
  def component_value
    send("#{component}").value
  end
  # it would be better to replace component with component_type
  # and have a method that just returns the component
end

object.constraints.each do |c|
  p = object.particles[c.index]
  if c.dir == :inf
    p.component_value = c.value if p.component_value < c.value
  else
    p.component_value = c.value if p.component_value > c.value
  end
end
2f6686bd2c6bb633a1cbf7b5a73c8491

micktaiwan

January 6, 2009, January 06, 2009 02:19, permalink

No rating. Login to rate!

Nice ! Thanks.

So I understand that "send" is what I needed.
component is a 3D vector component (x,y or z).
c.dir is a comparison function (:inf for inferior "<", and :sup for superior ">").

I had to add a "component" arg to the both functions, and change component_value= to component_set (or it won't work)
(But I should have submitted code that can be run and tested)
Much more compact now, thanks again.

class SomeObject
  def component_set(component,value)
    send("#{component}=", value)
  end
  
  def component_value(component)
    send("#{component}")
  end
end

object.constraints.each do |c|
  p = object.particles[c.index]
  if c.dir == :inf
    p.component_set(c.component,c.value) if p.component_value(c.component) < c.value
  else
    p.component_set(c.component,c.value) if p.component_value(c.component) > c.value
  end
end
2f6686bd2c6bb633a1cbf7b5a73c8491

micktaiwan

January 6, 2009, January 06, 2009 02:32, permalink

No rating. Login to rate!

And by replacing :inf or :sup, by :< or :> I can even replace the c.dir test completely.
So great !

Thanks Daniel for teaching me how to use Object#send.

object.constraints.each do |c|
  p = object.particles[c.index]
  p.component_set(c.component,c.value) if p.component_value(c.component).send(c.dir, c.value)
end
A8d3f35baafdaea851914b17dae9e1fc

Adam

January 6, 2009, January 06, 2009 06:34, permalink

2 ratings. Login to rate!

I think you need to rethink your data model. I'll include the following to give you some ideas, but it probably isn't the best way to go about it. I just don't have the information necessary to recommend the approach you probably should take.

class Constraint
  def particle
    ParticleConstraint.new(@owner.particles[index], self)
  end
end

class ParticleConstraint < Struct.new(:particle, :constraint)
  def method_missing(method, *args, &block)
    [ particle.send(method), constraint.value ].send(direction)
  end
  
  protected
    def direction
      constraint.dir == :inf ? :min : :max
    end
end

# === Example usage ===
object.constraints.each do |constraint|
  constraint.particle.x # => the value of X
  constraint.particle.y # => the value of Y
  constraint.particle.z # => the value of Z
end
Ab528fdbf1bb6b3a24f7d1a8e4216c1d

Piyush Patil

January 23, 2009, January 23, 2009 18:37, permalink

No rating. Login to rate!

Will this be helpful to You ?

object.constraints.each do |c|
  p = object.particles[c.index]
  component = (c.component == :x) ? 'x' : ((c.component == :y) ? 'y' : 'z')
  if c.dir == :inf
   p.send(component) = c.value if p.send(component) < c.value
  else
   p.send(component) = c.value if p.send(component) > c.value
  end
end
2f6686bd2c6bb633a1cbf7b5a73c8491

micktaiwan

January 23, 2009, January 23, 2009 20:16, permalink

No rating. Login to rate!

Thanks but Daniel solved my problem :)

1646ee10bae54b6e96a88ae757b43fd6

darius

January 24, 2009, January 24, 2009 16:11, permalink

No rating. Login to rate!

thanks. That exchange was informative.

also also:
I know the "elsif" clauses have been eliminated from this particular code, but you might also be interested to know that you can look for ranges using "case" statements. http://www.tutorialspoint.com/ruby/ruby_if_else.htm

Your refactoring





Format Copy from initial code

or Cancel