Why doesn't this ternary operator work (Ruby)?

Question

I'm trying to write program that computes factorial. However, when I try to run the code below, I get this error: undefined method `*' for nil:NilClass (NoMethodError)

1.upto(number) {|x| a = x==1 ? 1 : a*x }

Am I setting up the ternary operator incorrectly, or is something else wrong?

Thanks for the help

Answer 1

I'd do it something like:

def factorial(number)
  (2 .. number).inject(1) { |m, n| m * n }
end

factorial(1) # => 1
factorial(2) # => 2
factorial(3) # => 6
factorial(5) # => 120

inject is a useful method for things like this, and isn't limited to use with numbers.

It can be written even more concisely:

(1..n).inject(:*) || 1

which comes from "Ruby factorial function". That'll give you something to chew on for a while.

Your code is doing several things that aren't correct:

• a isn't ever defined prior to running, so a*x is doomed to fail because you can't multiply a nil by x.
• upto will pass the current value into the block as x, but assigning to a will fail because it's always a new local variable a. There is no static memory of a unless you define it outside the block's scope.
• In a lot of Ruby iterators, the value of the block can be used as a return value. upto doesn't work that way. You'll get the seed value returned, so, you'd get back 1.

Working out these sort of problems is best done using IRB, which comes with Ruby. Inside the interactive session you can try variations on your code to see what works. It's a lot faster/easier and more convenient than trying to write a script and go through the edit/run cycle.

---

What about this:

factorial = Hash.new{ |x,y| x[y]= y<2 ? 1 : x[y-1]*y }

Let's test that out by dropping into IRB to see what it does:

>> factorial = Hash.new{ |x,y| x[y]= y<2 ? 1 : x[y-1]*y }
{}
>> factorial[1]
1
>> factorial
{
    1 => 1
}
>> factorial[2]
2
>> factorial
{
    1 => 1,
    2 => 2
}
>> factorial[100]
93326215443944152681699238856266700490715968264381621468592963895217599993229915608941463976156518286253697920827223758251185210916864000000000000000000000000
>> factorial
{
      1 => 1,
      2 => 2,
      3 => 6,
      4 => 24,
      5 => 120,
      6 => 720,
      7 => 5040,
      8 => 40320,
      9 => 362880,
     10 => 3628800,
    ...
    100 => 93326215443944152681699238856266700490715968264381621468592963895217599993229915608941463976156518286253697920827223758251185210916864000000000000000000000000
}

YOW! You're going to compute every intermediate value!? It's going to rapidly consume memory, with very little advantage.

Answer 2

Your ternary operator is set up correctly, but your variable a is not defined at the point where you do the multiplication.

This will work because b has a value:

b = 5

1.upto(number) {|x| a = x==1 ? 1 : b*x }