Refactor this code to improve readability

Question

Please take a look at the below code

def test
  array = Array.new
  array2 = Array.new

  groups = [[424235, "goa", "italy"], [523436, "mumbai"], [342423, "africa", "goa"]]
  type = ["goa", "mumbai"]
  groups.each_with_index do |item,index|

       if item.include?(type[0]) == true
         array << index  << array2
        elsif item.include?(type[1]) == true
               array2 << index 
       else
         "nothing ;)"
       end

  end
  print array.each_slice(2).map { |a, b| [a, b.first] }
end
combine

#Output - [[0, 1], [2, 1]]

See the problem with the code? That is I am using a bunch of if and else statements. What if type array has more than 2 entries. I cant go on writing the if and elsif statements. And thats where I need your help. What is a better what to structure the code? loops? if so how.

What are you actually trying to do here? get the index's of types in groups? — Ahmed Aeon Axan
– Ahmed Aeon Axan, Commented Feb 17, 2013 at 12:31
yes. So I am looking for type[goa] and type[mumbai] in groups. And result them in the index of the location. Note that goa is always in precedence. — psharma
– psharma, Commented Feb 17, 2013 at 12:33
shouldnt your output then be [[0, 2], [1]]. Since goa is in group 0 and 2 and mumbai is only in group 1? — Ahmed Aeon Axan
– Ahmed Aeon Axan, Commented Feb 17, 2013 at 12:36
right. But its looking for the combination for the possible values in type against groups. So it is goa,mumbai(0,1) and goa,mumbai again but this time its 2,1. All possible combinations. Hope I am making sense :) — psharma
– psharma, Commented Feb 17, 2013 at 12:37

Ahmed Aeon Axan · Accepted Answer · 2013-02-17 13:15:49Z

2

Here is my code.

def combinations(groups, types)
  array = Array.new(types.size) { Array.new([]) }
  groups.each_with_index do |item, index|
     types.each_with_index { |type, i| array[i] << index if item.include? type }
  end

  flat = array.inject { |acc, i| acc.product i }.flatten
  flat.each_slice(types.size).to_a
end

Sample test cases

combinations([[424235, "goa", "italy"], [523436, "mumbai"], [342423, "africa", "goa"]], ["goa", "mumbai"])

output : [[0, 1], [2, 1]]

combinations([[424235, "goa", "italy"], [523436, "mumbai"], [342423, "africa", "goa"]], ["goa", "africa"])

output : [[0, 2], [2, 2]]

combinations([[424235, "goa", "italy"], [523436, "mumbai"], [342423, "africa", "goa"], [123, "india"]], ["goa", "mumbai", "india"])

output : [[0, 1, 3], [2, 1, 3]]

combinations([[424235, "goa", "italy"], [523436, "mumbai"], [342423, "mumbai", "goa"], [123, "india"]], ["goa", "mumbai", "india", "italy"])

output : [[0, 1, 3, 0], [0, 2, 3, 0], [2, 1, 3, 0], [2, 2, 3, 0]]

If I understood your problem correctly then these should be correct. Though I might have misunderstood you. Please do tell me If i have got your problem wrong, and If you could provide test cases that would be great.

answered Feb 17, 2013 at 13:15

Ahmed Aeon Axan

2,1391 gold badge18 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

psharma Over a year ago

nope I believe you did it right. I just need to tweak it a little to fit my updated case :). Thanks so much @ahmed

Collectives™ on Stack Overflow

Refactor this code to improve readability

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related