Displaying the multiples of a given number

Here's a method I made that's supposed to print out multiples of any numbers up to any quantity:

def DisplayMultiples(multiplesOf, count)
  i = multiplesOf
  while i <= count
    if i % multiplesOf == 0
      puts i
    end
    i += 1
  end
end

Any suggestions on how to improve the code to something more fitting to the Ruby style? I'm coming from a C# background so I'd like to switch things up a bit.

One important note regarding naming in ruby:

The syntax in ruby is that constants (including class and module names) have to start with capital letters and local variables have to start with lower case letters. Instance, class and global variables must start with their respective sigil followed by a letter in any case. Method names must start with a letter in any case, but if they start with a capital letters, they must be called with () when there are no arguments.

The naming convention in ruby is to use snake_case for method names and variables, PascalCase for class and module names and ALL_CAPS for constants other than class and module names.

For method names the convention is to use snake_case for all methods except "constructor"-style functions (e.g. Integer(), Array(), Hash()) where the name of the function is the same as that of the constructed class.

Generally using PascalCase for method names is a bad idea because:

• It's against convention and will make people think it's a constructor-style function

• If you define a method without arguments, you'll trip yourself up because you'll try to call it as MyMethod instead of MyMethod() which does not work if the method name starts with a capital letter.

General notes on your code:

Using a while loop is usually unidiomatic ruby. Using a while loop to iterate from one number to another number with a constant step-size is always unidiomatic ruby. If the step-size is 1, you can just use each on a range and if it is something else you can use step. So your code would just be written as:

def display_multples(multiples_of, count)
  (multiples_of .. count).each do |i|
    if i % multiples_of == 0
      puts i
    end
  end
end

Now as pdr points out, this is not the optimal algorithm to do this. Instead of iterating between all integers between multiples_of and count and printing those which are divisible by multiples_of, you can just iterate between the numbers between multiples_of and count with a step size of multiples_of, i.e. only iterate over the multiples of multiples_of. However his code contains an unnecessary each (which will also make the code not work on ruby 1.8.6, though that's thankfully becoming less of an issue these days). So the code should just be:

def display_multiples(multiples_of, count)
  (multiplesOf..count).step(multiples_of) do |i|
    puts i
  end
end

However this code still contains one major, non-ruby-specific design-smell: You're mixing the logic to generate the multiples with the code printing it. Generally IO and logic should be separated as much as possible. So what you should do is define a method multiples which generates the multiples and a method print_multiples which prints them.

In ruby the best way to write a method which generates a sequence of values is to write an "iterator method", i.e. a method which yields the elements. That would look like this:

def multiples(multiples_of, count)
  (multiples_of .. count).step(multiples_of) do |i|
    yield i
  end
end

def print_multiples(multiples_of, count)
  multiples(multiples_of, count) do |i|
    puts i
  end
end

You can also define multiples as:

def multiples(multiples_of, count, &blk)
  (multiples_of .. count).step(multiples_of, &blk)
end

In 1.8.7+ this also has the benefit of returning an Enumerator if no block is given. so if you don't just want to print it, you can also use Enumerable methods like map, select etc. on it.

StackExchange Code Review Q#803, answer score: 32