I have a case that looks like this :

John has 120 candies, He needs some plastic bags and marks each plastic bag with the number and puts 50 candies in each bag. So John can only fill up to 3 plastic bags :

plastic bag 1 = 50 candies

plastic bag 2 = 50 candies

plastic bag 3 = 20 candies

I want to create a Hash from the above case. The first variable is 120 candies and second variable is 50 candies for each plastic bag. I have written a method to create a Hash like that :

def make_a_hash(candies, each_candies)
  mark_number = 1
  arry = []

    put_candy = candies / each_candy > 0 ? each_candy : candies
    candies = candies - put_candy
    arry << [mark_number, put_candy]
    mark_number += 1
  end while candies > 0

#=> make_a_hash(120, 50)
#=> {1=>50, 2=>50, 3=>20}

That method is working, and I would like to know if there is another best practice instead my method.


The first remark I'd like to make about your function is that the names of things are not very clear as to what it is supposed to do. Imagine presenting only the code to someone, who doesn't know what problem you are trying to solve. Do you think it would be very clear?

We can make it more general, so that it can be applied to other problems. Instead of candies and plastic bags, let's use units and containers. That way, we can also calculate practically any "number of \$x\$ in \$y\$..." problems that involve integers:

  • Number of boxes in delivery trucks

  • Number of passengers in train carts

  • Etc.

Since we are looking to get as many full containers as we can, and then the remainder in another container, let's name the function that way as well. Here is what it looks like so far, with just the names changed:

def get_full_containers_and_remainder(units, container_capacity)
  container_number = 1
  containers = []

    units_in_this_container = units / container_capacity > 0 ? container_capacity : units
    units -= units_in_this_container
    containers << [container_number, units_in_this_container]
    container_number += 1
  end while units > 0

get_full_containers_and_remainder(120, 50) 
# {1=>50, 2=>50, 3=>20}

So far, so good. The logic is identical, but the function is easier to understand with more general names.

The logic can be simplified to not having to rely on manually adding units one container at a time in a loop. For such small numbers this is trivial, but if you had to divide up really big numbers this would take very long.

We can instead use a bit of simple math to calculate the remainder as well as the number of containers without a loop.

Using the modulo operator we can get the remainder:

remainder = units % container_capacity # remainder of 120 ÷ 50 = 20

Then we can calculate the number of full containers by subtracting the remainder from the total units, and then dividing by the container size.

count_full_containers = (units - remainder) / container_capacity # (120 - 20) ÷ 50 = 2

From there it's simply a matter of adding that many full containers, and then one more container with the remainder. Note that I added a check at the beginning to return an empty hash early if either of the values is zero, to avoid division-by-zero errors.

Working demo on repl.it

def get_full_containers_and_remainder(units, container_capacity)
  if units == 0 || container_capacity == 0
    return [].to_h

  remainder = units % container_capacity
  count_full_containers = (units - remainder) / container_capacity 
  container_number = 1
  containers = []

    containers << [container_number, container_capacity]
    container_number += 1
  end while container_number <= count_full_containers
  containers << [container_number, remainder]
  return containers.to_h

puts get_full_containers_and_remainder(120, 50) # {1=>50, 2=>50, 3=>20}
puts get_full_containers_and_remainder(0, 50)   # {}
puts get_full_containers_and_remainder(120, 0)  # {}

I think we can easily get division result and remainder with divmod. After that I would construct array with full groups and then add to it last group if it exists(reminder). Then there is left to just create a hash with indexes as keys from array.

def in_groups_with_index(num, capacity)
  full_groups, last_group_amount = num.divmod(capacity)
  array = [capacity] * full_groups
  array << last_group_amount if last_group_amount > 0
  array.each_with_index.inject({}) do |hash, (element, index)|
    hash.merge({(index+1) => element})

Method examples:

in_groups_with_index(120, 50) # => {1=>50, 2=>50, 3=>20}
in_groups_with_index(0, 50) # => {}
in_groups_with_index(120, 0) # => divided by 0 exception

For me it's strange to try to put 120 candies into bags that cannot carry any candies. But if you want method not throw exception in that case, you can add rescue in the and of method, or in the row where divmod is called.

full_groups, last_group_amount = (num.divmod(capacity) rescue [0,0])
# or
full_groups, last_group_amount = num.divmod(capacity) rescue return {}
# or
def in_groups_with_index(num, capacity)
  # ...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.