Hacker News Reader

Comment by ashray | Hacker News Reader

Is it just me or does his "sample" program look needlessly complex ? Does he really need to create all those function definitions ? I'm not super familiar with ruby but those ARE function definitions - right ?

Since when is it good programming practice to have functions for every little thing ? (I'm picturing myself scrolling or jumping around in an editor to figure out what exactly the call does). This is like doing hello world with a function. Or having a function to add two numbers. Yes, sometimes it's necessary for syntactical clarity but in this case it just seems like overkill.

Also why are some people on HN talking about edge cases to FizzBuzz ? There are no edge cases that I know of unless you talk about the number zero - which isn't part of the question.

EDIT: Why the downvotes? I don't see why trying to discuss what's clearly a programming related problem on HN brings you downvotes ? Interesting.

View on HN

> Also why are some people on HN talking about edge cases to FizzBuzz ? There are no edge cases that I know of unless you talk about the number zero - which isn't part of the question.

I don't know about you, but I sure see it as an edge case.

    # This is incorrect due to edge case.
    # If divisible by 3, fizz
    # If divisible by 5, buzz
    # If divisible by 15, fizz buzz
    # Else number
        1.upto(20) do |i|  
        if i % 3 == 0
            puts "Fizz"
        elsif i % 5 == 0
            puts "Buzz"
        elsif i % 15 == 0
            puts "FizzBuzz"
        else
            puts i
        end
    end

In my book, that's a pass, but you should have a discussion where you walk through the code and ask the interviewee what happens when i is 15, and hopefully they'll spot the problem, maybe with a little bit of nudging.

But the main point of Fizzbuzz is that some people who call themselves programmers are incapable of producing a loop, some conditionals, and some print statements. Your solution had that, which is why you passed it. The point is to find the people who can't even get that part right.

I see an incorrect algorithm in your solution but I don't really see what would be called an edge case (just to explain: anything divisible by 3 will enter the first conditional and omit the last, hence the algorithm you've used is incorrect). Or maybe we're talking about different things. Could you please explain to me in words what a fizzbuzz edge case would be ?

Yes. Who would give functions such cryptic names as "multiple_of_3" or "multiple_of_5".

One of the good things about this is it's easy to change the definition of "multiple_of_3" to reflect changing requirements. For example, suppose we wanted Fizz to be printed with multiples of 7 instead. Easy! Just change:

    def multiple_of_3(number):
       number % 3 == 0
    end

    def multiple_of_3(number):
       number % 7 == 0
    end

Now perhaps he sees the advantage: no refactoring necessary.

(* I feel I should say that I am being sarcastic. Anyone who thinks it's ok to have a function called "multiple_of_3" that checks if it is a multiple of 7 should probably be fired immediately.)

Yes, I see your point. But in this case if I wanted to change it to do it on 7, I'd have to make a change in 3 places.

1. The function definition (def multiple_of_3)

2. The function contents (number % 3 == 0)

3. The function call(s).

If I didn't try to use functions on this, I'd just have to change a single line (or two at the most) in my program and get on with it.

Your use case is exactly what I meant when I said that breaking such a simple problem into functions actually introduces complexity.

Yeah. Those functions should've been defined as

  def fizz?(i)
    i % 3 == 0
  end

  def buzz?(i)
    i % 5 == 0
  end

So that he'd just need to change them once.

My question was: Why would you create a function that does something you could access as a built in language feature ? What is the benefit of using a function as opposed to just evaluating the result of i%3==0 ?

Looking at a function call for "multiple_of_3" I would actually expect it to do more than i%3==0 since that could've been done just right there.

I'm just trying to understand if there are benefits of doing it this way and not directly. So far I've just encountered snarky remarks and no real arguments either way.

Before I quit The Daily WTF, the joke answer to your question would be "because it's resilient against someone changing the value of 3."

Which could, of course, be avoided by:

  > 3.frozen?
   => false
  > 3.freeze
   => 3
  > 3.frozen?
   => true

Just in case, I'd better add: :)

Well that makes sense. This is common practice in embedded systems without a filesystem. ;)

I created the convenience functions "multiple_of_3" and "multiple_of_5" for a couple of reasons:

1. To make it clear to the interviewer that I "get" the problem. 2. To prevent myself from making any dumb mistakes. I'm only a half-decent coder, so I've made it a habit of putting such "Mollyguards" (http://en.wiktionary.org/wiki/molly-guard) in my code as a precaution. 3. To show a little Ruby coding style. The convention in Ruby is to favour lots of concise little routines over big-ass ones. See "Eloquent Ruby" for the guideline and Shopify's ActiveMerchant library for an example of such Ruby in actual production code.

> To show a little Ruby coding style. The convention in Ruby is to favour lots of concise little routines over big-ass ones.

Writing small, concise components(classes, methods, functions, modules...) is a generally a good practice in any language. However, moving i % 3 in a separate method is not a good practice. As another commenter pointed out, your names are hardly apt. It would better serve to name them fizz?(multiple_of_3) and buzz?(multiple_of_5). But even then, if all fizz? is doing is i % 3, let it be inline. True that in the future you might need additional processing for fizz?(not for this particular eg; I am talking real world), but re-factor it when you need it. It's not like if you have i%3 inline, it makes it harder to re-factor it into a fizz? method later.

Also, since you are talking ruby conventions, shouldn't your predicates be named multiple_of_3? instead of multiple_of_3?

Yeah, you've gotta immediately refactor that into a multiple_of function that takes an argument x and returns another function that takes an argument y and returns whether it's a multiple of x.

Ah, Currying... (and how Firefox spellcheck recognizes that as a word, but not spellcheck)

* On another note, I've seen other bizarre blind spots in experienced programmers. One time, when I was learning Ruby, I went to a Ruby "hack night" in SF where a bunch of people there explained that they had been hackers for a while. The group was given a challenge to implement C-style integers (with overflow/loop-around) using Ruby classes.

The session had "mentors" who were professional Ruby programmers (mainly with Rails).

So as I was writing my class, and implementing addition, I figured I would need to input another integer object and add its value. So I needed to know how to expose a member variable, and couldn't find the doc that said how, leading me to ask a mentor.

He was stumped. I was thinking, "WTF? You program Ruby all day but don't know how to write [what I later found was called] an accessor?" I wasn't asking for anything fancy, just the plain ol'

    def foo
      @foo
    end

Reply on news.ycombinator.com