Trouble with combined logic in ruby

Question

writing a simple monte carlo simulation of a neutron beam. Having trouble with the geometry logic (whether something is in one environment or another). My issue is that Ruby seems to be processing the conditions sequentially and keeping the first value it comes to.

The code below illustrates this quite nicely:

def checkPosition(*args)

  polyCylRad = 2.5
  polyCylFr = 15
  polyCylB = -2.0
  borPolyBoxL = 9.0 / 2
  pbCylRad = 3.0
  pbBoxL = 10.0 / 2
  cdBoxL = 9.5 / 2

  position = Array.new
  material = String.new

  args.each do |item|
    position << item.inspect.to_f
  end
  xSquared = position.at(0) ** 2
  ySquared = position.at(1) ** 2
  zSquared = position.at(2) ** 2
  modX = Math.sqrt(xSquared)
  modY = Math.sqrt(ySquared)
  modZ = Math.sqrt(zSquared)

  puts xSquared
  puts Math.sqrt(ySquared + zSquared) <= polyCylRad
  puts (position.at(0) >= polyCylB)
  puts (position.at(0) <= polyCylFr)
  puts (position.at(0) >= polyCylB)and(position.at(0) <= polyCylFr)
  puts (position.at(0) <= polyCylFr)and(position.at(0) >= polyCylB)

  puts zSquared


  polyCylinder = (Math.sqrt(ySquared + zSquared) <= polyCylRad)and((position.at(0) >= polyCylB)and(position.at(0) <= polyCylFr) )
  puts polyCylinder
  borPolyBox = ((modX <= borPolyBoxL)or(modY < borPolyBoxL)or(modZ <= borPolyBoxL)) and not((modX >= cdBoxL)or(modY >= cdBoxL)or(modZ >= cdBoxL)) and not(Math.sqrt(ySquared + zSquared) <= polyCylRad)
  puts borPolyBox
  cadmiumShield = ((modX <= cdBoxL)or(modY < cdBoxL)or(modZ <= cdBoxL)) and not((modX >= pbBoxL)or(modY >= pbBoxL)or(modZ >= pbBoxL)) and not(Math.sqrt(ySquared + zSquared) <= polyCylRad)
  puts cadmiumShield
  leadShield = ( ((modX <= pbBoxL)or(modY <= pbBoxL)or(modZ <= pbBoxL)) or ((position.at(0) <= ployCylFr)and(Math.sqrt(ySquared + zSquared) <= pbCylRad)) ) and not(Math.sqrt(ySquared + zSquared) <= polyCylRad)
  puts leadShield

  if (polyCylinder) : material = "poly"
  elsif(borPolyBox) : material = "borPoly"
  elsif(cadmiumSheild) : material = "cd"
  elsif(leadSheild) : material = "pb"
  elsif(material == nil) : position = Array.new
  end

  thisEnvironment = Array.new
  thisEnvironment << position << material
  puts thisEnvironment.at(0)
  puts thisEnvironment.at(1)
end

checkPosition(40, 0, 0)

call the code whatever you want, but give it *args as an argument (I am lazy and may want to add more args in the future) then call it with 3 floats, wrt the geometry set up in the logic and you'll see what I mean.

My question is: how do I get it to work like it should (ie evaluating the logic correctly) without a whole bunch of nested if's? (which is what I am about to remake, however it is a nightmare to read and memory is cheap.)

Answer 1

You've typed "Sheild" a few times where you probably meant "Shield"

In the context you're using them, you should be using && instead of and, || instead of or, and ! instead of not. The reason is that or and and have such a low precedence that they will cause your assignment operators to not work the way you want. For example,

a = b and c

evaluates as

(a = b) and c

Such that a is always assigned the value b, and then in the result is truthy, c is evaluated (and discarded). On the other hand,

a = b && c

evaluates as

a = (b && c)

Which is what you want in this code.

Beyond that, I would move all of this code into a class, so that I can create lots of little methods for things:

class PositionChecker

  def initialize(*args)
    @x, @y, @z = *args
  end

  def checkPosition
    ...
  end

end

Look for opportunities to replace local variables in checkPosition with method calls. For example, you could move borPolyBox into its own method (once all of the values it uses are methods of their own):

class PositionChecker
  ...
  def borPolyBox
    ((modX <= borPolyBoxL)||(modY < borPolyBoxL)||(modZ <= borPolyBoxL)) && !((modX >= cdBoxL)||(modY >= cdBoxL)||(modZ >= cdBoxL)) && !(Math.sqrt(ySquared + zSquared) <= polyCylRad)
  end
  ...
end

Once you've got all of these predicates as their own method, you can create a method to determine the material, like so:

def material
  [
    [:polyCylinder, 'poly'],
    [:borPolyBox, 'borPoly'],
    [:cadmiumShield, 'cd'],
    [:leadShield, 'pb'],
  ].each do |method, name|
    return name if send(method)
  end
  nil
end

And one for the position:

def position
  [@x, @y, @z] if material
end

Continue along this line until nothing is left but a bag of teeny, focused methods.

Answer 2

Change all and and or to && and ||.

Never seen anyone actually use array.at(index) instead of array[index] before.

I also recommend against *args in favor of a Hash parameter as a kind of named parameters

def test(params)
  x = params[:x] || raise("You have to provide x!")
  y = params[:y] || raise("You have to provide y!")
  z = params[:z] || raise("You have to provide z!")
  puts x, y, z
end

and call it with (Ruby 1.9+ syntax)

test({x: 42, y: 4711, z: 93})

42
4711
93

Answer 3

Try using && instead of and, || instead of or and ! instead of not.

Your issue is probably a precedence one - read this article for more information.

Answer 4

Looking at your code, the way you are setting this up is to have the four "material" variables as booleans. Then you feed those bools into an if-elsif-else block.

The problem with this is that the very first if that returns true is going to exit the if-elsif-else block. If that's what you mean by keeping the first value it comes to, than that's an extremely predictable outcome.