foo bar not printing the correct output

Question

Currently my code will display 1 through 100 but will not actually print out the case statements when it is divisible by 3 or 5 or both ....What am I doing wrong here?

def display(item_count)
  n = 1  
  while n <= item_count
    case n
    when n % 3 == 0 
      puts "foo"
    when n % 5 == 0
      puts "bar"
    when (n % 3 ==0 && n % 5 == 0) 
      puts "foobar"
    else
      puts "#{n}"
    end #--  case end stmt
    n = n + 1
  end #while end statement
end #end satement for metod

puts " How many items do you want to see"
item_count = gets.chomp.to_i
puts "#{display(item_count)}"

The exact purpose of the foobar problem was to detect/exclude people who write code like this. So indeed, the problem is functioning as intended. — sawa, Oct 25 '17 at 10:51
currently my code will display 1 through 100 but will not actually print out the case statements when it is divisible by 3 or 5 or both ....What am I doing wrong here ? — Nabeel A.Rahman, Oct 25 '17 at 10:55
Read about the [`case` expressions](https://ruby-doc.org/core-2.4.1/doc/syntax/control_expressions_rdoc.html#label-case+Expression) in Ruby until you find it obvious why the posted code always takes the `else` branch. — axiac, Oct 25 '17 at 10:59
@axiac ...ok I see what I did wrong ..Here is the new implementation: def display(item_count) n = 1 while n <= item_count case when (n % 3 ==0 && n % 5 == 0) puts "Foobar" when n % 5 == 0 puts "Bar" when n % 3 ==0 puts "Foo" else puts "#{n}" end #-- case end stmt n = n + 1 end #while end statement end #end satement for metod puts " How many items do you want to see" item_count = gets.chomp.to_i puts "#{display(item_count)}" — Nabeel A.Rahman, Oct 25 '17 at 11:30
@NabeelA.Rahman: JFYI, multiline code snippets in comments are unreadable. Better post them as links to gist.github.com (or something) or add to the question (when appropriate). — Sergio Tulentsev, Oct 25 '17 at 11:31

Max · Answer 1 · 2017-10-25T12:29:06.657

1

The case expression compares the argument you give it with each of the when expressions using === (which is the same as == in most cases). So your code is roughly equivalent to

if (n % 3 == 0) === n
  puts "foo"
elsif (n % 5 == 0) === n
  puts "bar"
elsif (n % 3 == 0 && n % 5 == 0) === n
  puts "foobar"
else
  puts "#{n}"
end

All of the when expressions return true or false, so every case fails and you end up in the else. Just use this if format instead of case and you can do what you want (though you'll need to rearrange your conditions to get the right output).

edited Oct 25 '17 at 12:29

answered Oct 25 '17 at 12:28

Max

21,123
5
49
71

@SergioTulentsev I'm not trying to post a complete working solution, if you actually read my answer. I'm showing the OP what logic his code is actually performing. :) – Max Oct 25 '17 at 12:31
Ah, sorry, my bad :) – Sergio Tulentsev Oct 25 '17 at 12:32

score 1 · Accepted Answer · answered Oct 26 '17 at 02:15

You can use a case statement in ruby without an argument. The when clause must still return true or false.

case
when true
  puts 'this happens'
when false
  puts 'this does not happen'
end

Therefore, the code in the original post will work (at least, the case statement) if you simply change the

case n

into

case

and move the 'foobar' test to the top of the "when" list. My other answer also uses this construct, but eliminates the stack manipulation and yields a storable/transmittable object instead of merely displaying results.

Note: calling the zero? method on an object is faster than comparing it to a literal 0. (x.zero? is faster than x == 0).

"must still return true or false" - not quite. Just truthy/falsy, as it is in most of ruby. — Sergio Tulentsev, Oct 26 '17 at 11:48
You are correct, and that is an important distinction to make. — Tom, Oct 27 '17 at 18:13

score 0 · Answer 3 · answered Oct 25 '17 at 11:10

0

print "Enter amount of times> "
(1..gets.to_i).each do |i|
  puts(
    "".tap do |res|
      res << "foo" if (i % 3).zero?
      res << "bar" if (i % 5).zero?
      res << i.to_s if res.empty?
    end)
end

answered Oct 25 '17 at 11:10

Aleksei Matiushkin

119,336
10
100
160

1

Very functional, not very readable. :) Let's see if I can come up with a better snippet. – Sergio Tulentsev Oct 25 '17 at 11:22
[Extensible](https://pastebin.com/iyYAkAbL) solution. Although even less readable. :/ – Sergio Tulentsev Oct 25 '17 at 11:30
@SergioTulentsev I had exactly this in the first version, but later I decided that looping 2 elements does not make any sense :) Of course, `case` is a way to go here. – Aleksei Matiushkin Oct 25 '17 at 11:36
I thought I had a [beginning of a smart-ass solution](https://pastebin.com/DhUAgek7), but can't figure out how to use that with `.grep` (in a nice clean way). Oh well... – Sergio Tulentsev Oct 25 '17 at 11:48
@SergioTulentsev like that: `(1..gets.to_i).map { |i| filters.map { |f, v| v if f === i }.join.tap { |e| e << i.to_s if e.empty? } }`? It could be shorter with a temporary local variable for `filters.map.compact`, but I hate local variables. – Aleksei Matiushkin Oct 25 '17 at 11:53
The idea was to use something that will hide the `===` and therefore look like magic. `grep` is a good example, `case` is another. – Sergio Tulentsev Oct 25 '17 at 11:57
Check this out! https://gist.github.com/stulentsev/b7fc3830c9d21a115308f275111cf549 – Sergio Tulentsev Oct 25 '17 at 12:08
@SergioTulentsev Monadic composition! I like it. – Aleksei Matiushkin Oct 25 '17 at 12:14
This complete re-engineering of the solution does nothing to answer OP's question. – Max Oct 25 '17 at 12:33
@Max _“Any answer that gets the asker going in the right direction is helpful”_ —[here](https://stackoverflow.com/help/how-to-answer). – Aleksei Matiushkin Oct 25 '17 at 12:41

Tom · Answer 4 · 2017-10-27T18:25:19.047

-1

Displaying something is one thing; having the result of a function available in memory for further use, such as transport over a network connection or storage, is something else. In ruby everything is an object, so you should operate on and with objects. This is what ruby was made for.

Edited to address looping multiple times and readability.

My original was decried as unreadable, and then someone came up with a monadic solution using a lambda literal and monkey patching. Even a seasoned programmer will be challenged to follow its logic, and the stack manipulation involved in the interpreter throws scalability out the window anyway. A procedural block in a single loop addresses all of that without being pedantic. "Readable" also implies "understandable."

Freezing the string literals should not be necessary in ruby >= 2.4; they should be automatically frozen, but since the single-loop solution is to address scalability (imagine calling it a million times), one should not overlook this optimization. Also, I have reduced it to a single conditional statement within the block, which means greater scalability.

print 'How many items do you want to see: '.freeze

list = (1..gets.to_i).collect do |i|
  div_by_3 = (i % 3).zero?
  div_by_5 = (i % 5).zero?
  case 
    when div_by_3 && div_by_5
      :foobar
    when div_by_3
      :foo
    when div_by_5
      :bar
    else
     i
  end
end

puts list.to_s

Output:

How many items do you want to see: 50
[1, 2, :foo, 4, :bar, :foo, 7, 8, :foo, :bar, 11, :foo, 13, 14, :foobar, 16, 17, :foo, 19, :bar, :foo, 22, 23, :foo, :bar, 26, :foo, 28, 29, :foobar, 31, 32, :foo, 34, :bar, :foo, 37, 38, :foo, :bar, 41, :foo, 43, 44, :foobar, 46, 47, :foo, 49, :bar]

edited Oct 27 '17 at 18:25

answered Oct 25 '17 at 11:24

Tom

412
2
9

2

This is **not** a ruby way by any means. Three subsequent loops instead of the only one required immediately strikes this code out from being acceptable. – Aleksei Matiushkin Oct 25 '17 at 11:36
The real world doesn't work like that; your objection is pedantic outside the scope of scalability. Silicon is cheap compared to programmer time. If scalability were a primary concern, you could in fact do the same thing with a single map.with_index, but it would be ugly. Ruby code is most often used to transport results over a network, or to transform stored data and store it again. For that you must put your results in a data structure. – Tom Oct 25 '17 at 11:45
2

This code is completely unreadable as well (read: it’s a perfect example of how one _should never ever write ruby_.) Mutating lists inplace, three unnecessary ternaries, redundant checks: all that smells. – Aleksei Matiushkin Oct 25 '17 at 11:46
_Sidenote:_ you do not need `to_a` in the initial line, ranges are iterable as well. – Aleksei Matiushkin Oct 25 '17 at 11:49
Although I would have worded it diffferently, I will have to agree with mudasobwa. This is not what idiomatic ruby looks like. Out of all snippets in this topic, the one in the question is closest to ruby way (needs some fixes, obviously. Like separating printing from calculation) – Sergio Tulentsev Oct 25 '17 at 11:51
What I mean by 'the ruby way' is that everything is an object, so why would you write code that does not operate on an object, let alone one you can't use for anything? – Tom Oct 25 '17 at 11:56
@Tom: with that I agree, printing should be extracted. – Sergio Tulentsev Oct 25 '17 at 11:59
Thanks. That's the main thing I was going for. Beyond that, I did learn something; I had not seen tap before. Good stuff. – Tom Oct 25 '17 at 12:02
Much better, this. Only now your `map` returns mixed data types. Do you think this is a problem or is it intended? – Sergio Tulentsev Oct 26 '17 at 11:52
And you're right, of course. My monadic solution is the slowest of the bunch, but, arguably, most beatiful :) – Sergio Tulentsev Oct 26 '17 at 12:08
The mixed data types were actually intentional, and the original answer had that as well. Ruby is a dynamically typed language, there is no reason to do otherwise. Enumerating the array later can duck-type individual values if necessary. Come to think of it, it would also be more efficient to use symbols :foo, :bar, and :foobar rather than strings, not just for the ruby virtual machine and the algorithm but for storage as well. – Tom Oct 27 '17 at 18:16

foo bar not printing the correct output

4 Answers4