Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Just for the sake of understanding. I know JSON.parse could do this pretty easily, but without using that, how could this be made cleaner or more efficient?

string = "ounces: 10; contains_toy: false"
result = string.split("; ")
result = result.map{|x| x = x.split(": "); Hash[x.first.to_sym, x.last] }
result = result.reduce(:merge)

Desired result:

{:ounces=>"10", :contains_toy=>"false"}
share|improve this question
1  
Also for the sake of understanding: How would JSON.parse help? The string isn't close to being JSON. Keys would have to be quoted, commas instead of semicolons, braces around everything. – Flambino yesterday
    
I was thinking something like this : result = JSON.parse "{#{string.gsub(/([a-z_]+):/, '"\1":')}}" was a little cleaner – newUserNameHere yesterday
1  
With gsub + parse you'd probably hit the same issue that any sort of naïve string manipulation (including the code in my answer) has: Delimiter-tokens being taken taken out of context, and breaking things. – Flambino 23 hours ago

1 Answer 1

up vote 4 down vote accepted
  1. Chain your calls. Don't update the same variable again and again with very different values.

  2. Just construct a hash straight away with each_with_object or construct an array of pairs, and turn that into a hash.

  3. Any sort of splitting/scanning is prone to errors if the string contains tokens that aren't part of the syntax. I.e. a nested string with a semicolon in it will be split as though it's a delimiter.

Anyway, you can do this, for instance:

result = Hash[ string.scan(/(\w+):\s+([^;]+)/) ]

though that'll give you string keys.

To avoid that, you could do:

pairs = string.scan(/(\w+):\s+([^;]+)/).map { |k,v| [k.to_sym, v.strip] }
result = Hash[pairs]

or

result = string.scan(/(\w+):\s+([^;]+)/).each_with_object({}) do |(k,v), hash|
  hash[k.to_sym] = v.strip
end

Another, much less robust way would be:

pairs = string.split(/[:;]/)
  .map(&:strip)
  .each_slice(2)
  .map { |k,v| [k.strip.to_sym, v.strip] }
result = Hash[pairs]

Edit: And the totally unsafe option would be

result = eval("{#{string.tr(';', ',')}}")

On the plus side, your values have proper types (not all strings). On the minus side, eval is evil, and should always be avoided. So, really, don't do this.

share|improve this answer
    
Thanks for the detailed explanation as well as the variations! Great explanation :) – newUserNameHere yesterday
1  
@newUserNameHere Thanks - added another (really poor) alternative. But it's the shortest one yet. – Flambino 23 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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