0

I was working on coding challenge called Robot name. I also had tests for that. The program passed all the tests. The code is below..

class Robot
  attr_accessor :name
  @@robots = []
  def initialize
    @name = self.random_name
    @@robots << self.name
  end

  def random_name
    name = ''
    2.times do
      name  << ('a'..'z').to_a.sample
    end
    3.times do
      name  << (1..9).to_a.sample.to_s
    end
    no_duplicate(name.upcase)
  end

  def reset
    @name = self.random_name
  end

  def no_duplicate(name)
    if @@robots.include? name
      reset
    else
      name
    end
  end
end

If you need to see the tests file you can look it up here robot_name_tests.

Then I started to refactor and one of the first things was to refactor no_duplicate method. So after refactoring the code looked like this

class Robot

  ...
  # the rest of code stayed the same

  def no_duplicate(name)
    @@robots.include? name ? reset : name
  end
end

With this version all tests showed SystemStackError: stack level too deep. Why does it give this error and what is going on behind the scenes in both cases considering the code provided? Thanks!

buterfly85
  • 163
  • 3
  • 11

1 Answers1

5

I like your poetry mode code but it has led you into trouble here.

One way to kinda keep it in poetry mode but fix your operator priority issue is to do this:

   def no_duplicate(name)
     (@@robots.include? name) ? reset : name
   end

Update: if you work in Big Corporation With Coding Standards you will need to make it a bit more boring. I thought this was obvious but the gallery is correctly noting the usual solution:

   @@robots.include?(name) ? reset : name
DigitalRoss
  • 143,651
  • 25
  • 248
  • 329
  • That worked perfectly! Thanks! But why this confused ruby? because of `name`? It doesn't get confused every time when using this type of statement. Why in this case? – buterfly85 Jun 21 '16 at 00:31
  • Because the ? : operator bound more closely than the function parameters did. That is, Ruby was evaluating: `@@robots.include?(name ? reset : name)` – DigitalRoss Jun 21 '16 at 00:35
  • Got it!Thanks for explanation. – buterfly85 Jun 21 '16 at 00:37
  • 3
    A more idiomatic solution would be to simply use parentheses around the arguments: `@@robots.include?(name) ? ...` – Jordan Running Jun 21 '16 at 02:31
  • @Jordan: Yes! Pretty much every question about errors regarding the conditional operator can be answered either with "follow the Ruby Community Style Guide" or "use a conditional expression instead". Which is why I don't get why Rubyists love the conditional operator so much. I find `if @@robots.include? name then reset else name end` much more readable, *and* it has the precedence that everybody expects. The conditional operator is a crutch required in C, because `if` is a statement, not an expression, but in Ruby it *is*, and so the conditional operator is just superfluous. – Jörg W Mittag Jun 21 '16 at 07:36
  • @Jordan, I figured that would be obvious. At first glance it looked like she wanted poetry mode. Since I'm a poetry-mode nut I gave her the kinda-lispy answer. But, she wasn't consistent about poetry mode so maybe the garden-variety function call is best, sure. – DigitalRoss Jun 21 '16 at 20:27
  • Nothing is obvious to newcomers, least of all the meaning of jargon like "poetry mode." – Jordan Running Jun 21 '16 at 20:30