Ruby efficient each loop

Question

Is there a better way to write this? markdown is a StringIO

coverage_hash_arr = [
        {
          "Module": "Mobile",
          "name": "Sheila Chapman",
          "age": 21
        },
        {
          "Module": "Web",
          "name": "Hendricks Walton",
          "age": 40
        },
        {
          "Module": "Misc",
          "name": "Torres Mcdonald",
          "age": 39
        }
    ]

coverage_hash_arr.each do |the_hash|
    markdown << "------- Status on #{the_hash[:Module]} -------\n"
    the_hash.delete(:Module)
    the_hash.each {|key, value| markdown << "- #{key}: #{value} \n"} 
    markdown << "----------------------------------------------\n"
end

I tried this and it seems to work but I wonder if there's a better way (recursion)?

coverage_hash_arr.collect do |the_hash|
    the_hash.each do |key,value|
        key == :Module ? markdown << "--------- Status for #{value} ----------\n" : markdown << " - #{key}: #{value} \n"
      end
      markdown << "------------------------------------\n\n"
end

Hard to know when we do not know what coverage_hash_arr and markdown are. What is your input, what is the expected output? Btw are you really still on ruby-on-rails-3, are Rails version that is about 8 years old? — spickermann
– spickermann, Commented Dec 15, 2020 at 5:53
markdown is a StringIO (specified above) and coverage_hash_arr would be an array of hashes. — user2994871
– user2994871, Commented Dec 15, 2020 at 5:59
Remember in Ruby there's a huge difference between "Module" (String) and :Module (Symbol). These two are not equal. You get lucky here because "Module": x defines a Symbol key, the quotes are extraneous. — tadman
– tadman, Commented Dec 15, 2020 at 6:13

Stefan · Accepted Answer · 2020-12-15 09:12:17Z

You could:

use puts instead of << to avoid explicit newlines
use center to center the caption horizontally
use map to generate the attribute strings and utilize puts' behavior of printing array elements on separate lines
use without to get a hash without the :Module key
use * to repeat a string

Applied to your code:

markdown = StringIO.new

coverage_hash_arr.each do |hash|
  markdown.puts " Status on #{hash[:Module]} ".center(46, '-')
  markdown.puts hash.without(:Module).map { |k, v| "- #{k}: #{v}" }
  markdown.puts '-' * 46
  markdown.puts
end

Output via puts markdown.string:

-------------- Status on Mobile --------------
- name: Sheila Chapman
- age: 21
----------------------------------------------

--------------- Status on Web ----------------
- name: Hendricks Walton
- age: 40
----------------------------------------------

--------------- Status on Misc ---------------
- name: Torres Mcdonald
- age: 39
----------------------------------------------

Note that the above isn't proper Markdown syntax. You might want to change your output to something like this:

### Status on Mobile
- name: Sheila Chapman
- age: 21

### Status on Web
- name: Hendricks Walton
- age: 40

### Status on Misc
- name: Torres Mcdonald
- age: 39

tadman · Accepted Answer · 2020-12-15 06:17:58Z

2

Here's a more streamlined version which has been adapted to be more idiomatic Ruby:

# Define your hashes with keys having consistent case, and omit extraneous
# surrounding quotes unless defining keys like "this-name" which are not
# valid without escaping.
coverage = [
  {
    module: "Mobile",
    name: "Sheila Chapman",
    age: 21
  },
  {
    module: "Web",
    name: "Hendricks Walton",
    age: 40
  },
  {
    module: "Misc",
    name: "Torres Mcdonald",
    age: 39
  }
]

# Iterate over each of these elements...
coverage.each do |entry|
  markdown << "------- Status on #{entry[:module]} -------\n"

  entry.each do |key, value|
    # Skip a particular key
    next if (key == :module)

    markdown << "- #{key}: #{value} \n"
  end

  markdown << "----------------------------------------------\n"
end

This can be adapted to have a list of keys to exclude, or the inverse, of having a list of keys to actually print.

There's really nothing wrong with your approach specifically. The major faux-pas committed is in using delete on the data, which mangles it, rendering it useless if you needed to print this again.

It's generally best to try and avoid tampering with the data you're iterating over unless the purpose of that code is clear in its intent to alter it.

answered Dec 15, 2020 at 6:17

tadman

212k23 gold badges237 silver badges266 bronze badges

2 Comments

user2994871 Over a year ago

Thanks! I like the the skipping of a particular key. Is there a way to do this without the nested each loop?

tadman Over a year ago

You can replace it with an each-like iterator, such as map, but if you want to print out those keys in a specific form you're sort of stuck with some variation on that.

spickermann · Accepted Answer · 2020-12-15 08:17:58Z

2

It looks like your input data has always the same key/value pairs and I would be more explicit to make it easier to read and to understand what the actual output is:

coverage_hash_arr.each do |hash|
  markdown << <<~STRING
    ------- Status on #{hash[:Module]} -------
    - name: #{hash[:name]}
    - age:  #{hash[:age]}
    ----------------------------------------------
  STRING
end

answered Dec 15, 2020 at 8:17

spickermann

108k9 gold badges115 silver badges147 bronze badges

Collectives™ on Stack Overflow

Ruby efficient each loop

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related