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

score 3 · Answer 1 · edited Sep 12 '13 at 18:04

3

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 }

edited Sep 12 '13 at 18:04

the Tin Man

158,662
42
215
303

answered Sep 12 '13 at 18:00

RogerMo

88
6

Your code does not make much sense for calculating factorial. – sawa Sep 12 '13 at 18:26
Sorry, I was heading into a meeting. I was just trying to make the point that you can't use a because it's not defined. – RogerMo Sep 12 '13 at 20:26

score 2 · Accepted Answer · edited May 23 '17 at 11:56

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.

Thanks, that seems like the most concise method. What about this: factorial = Hash.new{ |x,y| x[y]= y<2 ? 1 : x[y-1]*y } — JordanD, Sep 12 '13 at 18:26
Regarding your concise version, `(1..n).inject(1, :*)` is cleaner. — sawa, Sep 12 '13 at 18:30
@JordanD, yes, how about that? Drop that into IRB and see if it works. Also, does it pass a simple sanity check of being readable/understandable? — the Tin Man, Sep 12 '13 at 18:34

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

2 Answers2