4
\$\begingroup\$

I have an array of Item objects that each have a string particular and a numeric amount. I want to put all particulars (separated by newlines) into a single string and the same for all amounts.

Here's what I came up with:

particulars = []
amounts = []

items.each do |item|
  particulars << item.particular
  amounts << item.amount
end

particulars_string = particulars.join("\n")
amounts_string = amounts.join("\n")

So if

item1.particular = "food"
item2.particular = "drink"
item1.amount = 1000
item2.amount = 2000

then running the code above gives

particulars_string # "food\ndrink"
amounts_string # "1000\n2000"

which is correct. However, I feel that the above code can be done better. In particular, I want all the code in one loop, not the three (each and two joins) I have now. What's a better way to do this?

\$\endgroup\$
4
\$\begingroup\$

You can get by with just two lines:

particulars_string = items.map(&:particular).join("\n")
amounts_string     = items.map(&:amount).join("\n")

Enumerable#map, which is mixed into Array, creates a new array from an existing array by running a block on each element and storing the result. In this case though you don't need a full block, since you just need to call a single method (particular or amount). So it basically extracts those into new arrays and then joins those arrays.

As a rule of thumb, you almost never have to "manually" map stuff from one array to another in Ruby using each and <<. Be sure to read the docs for Array and Enumerable as they're full of good stuff.

\$\endgroup\$
  • \$\begingroup\$ Exactly what I was looking for! \$\endgroup\$ – jcm Jan 15 '14 at 7:50
  • \$\begingroup\$ I don't really like this approach — you're still doing 4 enumerations of the length of the items array, and there's duplication on 2 local variables, 2 methods, and an argument. \$\endgroup\$ – coreyward Jan 20 '14 at 20:40
  • \$\begingroup\$ @coreyward True. Your use of transpose is more elegant than the duplication above. \$\endgroup\$ – Flambino Jan 20 '14 at 21:34
  • \$\begingroup\$ @Flambino, you may be mixing me up with coreyward. I think your solution is just fine. +1 \$\endgroup\$ – Cary Swoveland Jan 22 '14 at 7:29
  • \$\begingroup\$ @CarySwoveland Argh, so sorry. I was mixing you up - my mistake. But, then, your use of transpose is more elegant :) \$\endgroup\$ – Flambino Jan 22 '14 at 10:40
2
\$\begingroup\$

If you had several attributes (particular, amount, ...), you might consider doing it this way:

class Items
  attr_accessor :attributes
  def initialize(*attributes)
    @attributes = attributes
  end
end

items = [Items.new('food', 1000), Items.new('drink', 2000)]

attributes_strings = items.map(&:attributes).transpose.map {|e| e.join('\n')}

puts attribute_strings
food\ndrink
1000\n2000
\$\endgroup\$
  • \$\begingroup\$ Clever. I didn't know about transpose. \$\endgroup\$ – jcm Jan 16 '14 at 2:55
  • \$\begingroup\$ As mentioned: Nicer than my answer. However, you're assuming/adding some stuff compared to the original question where the item objects; it's not given that the objects in items respond to, or can feasibly be made to respond to, attributes. Luckily, it's not necessary. I'd shorten it to simply: particulars_string, amounts_string = items.map {|item| [item.particular, item.amount]}.transpose.map {|a|a.join("\n")}. Same input/result as the original with no assumptions about or additions to code outside of that in the original question. \$\endgroup\$ – Flambino Jan 20 '14 at 21:44
  • \$\begingroup\$ @Flambino, I'm not sure if I understand your point about items. The contents of items is code, not data. \$\endgroup\$ – Cary Swoveland Jan 22 '14 at 7:27
  • \$\begingroup\$ @CarySwoveland I just consider the objects in items "out of scope". All we know is that there's an array of items that each respond to particular and amount. You define your own Item class that responds to attributes, which may or may not be possible - we don't know enough about the context of OP's code. Of course you're more than welcome to suggest different approaches, but it's not necessary in this case, where it's a simple "given this array, produce two strings". And the core of your answer (map + transpose + map) is great for that even without assuming anything about the items \$\endgroup\$ – Flambino Jan 22 '14 at 10:57

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.