Forum: Ruby code refactoring/expressiveness

249c7fd851c5c5ac5a1abdb756472ae1?d=identicon&s=25 Arup Rakshit (my-ruby)
on 2014-03-11 09:47
The below code is completely working :

a = [{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}]
merged_hash = a.each_with_object({})  do |item,hsh|
  k,v = item.shift
  hsh.has_key?(k) ? hsh[k] = [v] << hsh[k] : hsh[k] = v
end

merged_hash.map { |k,v| { k => v } }
# => [{:a=>[10, 1]}, {:b=>8}, {:c=>[2, 7]}]

But, Is it a expressive code or some tweak can still be applied on this
?
E088bb5c80fd3c4fd02c2020cdacbaf0?d=identicon&s=25 Jesús Gabriel y Galán (Guest)
on 2014-03-11 10:34
(Received via mailing list)
On Tue, Mar 11, 2014 at 9:47 AM, Arup Rakshit <lists@ruby-forum.com>
wrote:
>
> But, Is it a expressive code or some tweak can still be applied on this
> ?

Although it's not exactly the same solution (single elements end up in
arrays too), I propose this:

2.0.0p195 :005 > a = [{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}]
 => [{:a=>1}, {:a=>10}, {:b=>8}, {:c=>7}, {:c=>2}]

2.0.0p195 :006 > merged_hash = Hash.new {|h,k| h[k] = []}
 => {}

2.0.0p195 :007 > a.each {|h| h.each {|k,v| merged_hash[k] << v}}
 => [{:a=>1}, {:a=>10}, {:b=>8}, {:c=>7}, {:c=>2}]

2.0.0p195 :008 > merged_hash
 => {:a=>[1, 10], :b=>[8], :c=>[7, 2]}

It might give you some ideas.

Jesus.
6e366eb5a71be2bad7f383d42aeb4788?d=identicon&s=25 Justin Collins (Guest)
on 2014-03-11 10:38
(Received via mailing list)
On 03/11/2014 01:47 AM, Arup Rakshit wrote:
>
> But, Is it a expressive code or some tweak can still be applied on this
> ?
>

This is slightly different than your example (so not a "true
refactoring" I guess). I don't modify the original array and all
"values" in the result are arrays, which seems more consistent:

# This is all just to avoid assigning k and v when iterating over values
a =
[{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}].map(&:to_a).map(&:flatten)

merged = Hash.new { |h, k| h[k] = { k => [] } }

a.each do |k, v|
   merged[k][k] << v
end

merged.values
# => [{:a=>[1, 10]}, {:b=>[8]}, {:c=>[7, 2]}]


No guarantees this code will be more efficient or better in any way.

-Justin
E0d864d9677f3c1482a20152b7cac0e2?d=identicon&s=25 Robert Klemme (robert_k78)
on 2014-03-11 11:13
(Received via mailing list)
On Tue, Mar 11, 2014 at 9:47 AM, Arup Rakshit <lists@ruby-forum.com>
wrote:
> The below code is completely working :
>
> a = [{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}]
> merged_hash = a.each_with_object({})  do |item,hsh|
>   k,v = item.shift
>   hsh.has_key?(k) ? hsh[k] = [v] << hsh[k] : hsh[k] = v
> end
>
> merged_hash.map { |k,v| { k => v } }
> # => [{:a=>[10, 1]}, {:b=>8}, {:c=>[2, 7]}]

Why do you want to have an Array with Hashes as result?  Since you are
grouping by key a single Hash seems like a much better choice.

irb(main):001:0> a = [{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}]
=> [{:a=>1}, {:a=>10}, {:b=>8}, {:c=>7}, {:c=>2}]
irb(main):002:0> merged = Hash.new {|h,k| h[k]=[]}
=> {}
irb(main):003:0> a.each {|h| h.each {|k,v| merged[k] << v}}
=> [{:a=>1}, {:a=>10}, {:b=>8}, {:c=>7}, {:c=>2}]
irb(main):004:0> merged
=> {:a=>[1, 10], :b=>[8], :c=>[7, 2]}

Kind regards

robert
249c7fd851c5c5ac5a1abdb756472ae1?d=identicon&s=25 Arup Rakshit (my-ruby)
on 2014-03-11 11:20
Anyhow I reached to this one :

a = [{:a=>1},{:a=>10},{:b=>8},{:c=>7},{:c=>2}]
merged_hash = a.each_with_object({})  do |item,hsh|
  k,v = item.shift
 (hsh[k] ||= []) << v
end

p merged_hash.map { |k,v| { k => v } }
# => [{:a=>[10, 1]}, {:b=>8}, {:c=>[2, 7]}]
249c7fd851c5c5ac5a1abdb756472ae1?d=identicon&s=25 Arup Rakshit (my-ruby)
on 2014-03-12 11:32
One more refactoring :

All is working.

hash = {
  "properties"=>{
    "one"=>"extra",
    "headers"=>{
      "type"=>"object",
      "type1"=>"object2"
    },
    "entity"=>{
      "type"=>"entype"
    },
  },
  "sec_prop"=>"hmmm"
}

def fetch_keys(h)
  h.flat_map do |k,v|
    if v.is_a?(Hash)
      fetch_keys(v).map do |keys|
        [k] + Array(keys)
      end
    else
      k
    end
  end
end

fetch_keys(hash)
# => [["properties", "one"],
#     ["properties", "headers", "type"],
#     ["properties", "headers", "type1"],
#     ["properties", "entity", "type"],
#     "sec_prop"]

Just thinking if the below can be done in any other way ?

      fetch_keys(v).map do |keys|
        [k] + Array(keys)
      end
E0d864d9677f3c1482a20152b7cac0e2?d=identicon&s=25 Robert Klemme (robert_k78)
on 2014-03-12 13:20
(Received via mailing list)
On Wed, Mar 12, 2014 at 11:32 AM, Arup Rakshit <lists@ruby-forum.com>
wrote:
> One more refactoring :
> [...]
> Just thinking if the below can be done in any other way ?

You keep changing structure and contents of your data structure. It's
impossible to determine what your goal is so it's hard to come up with
suggestions. I suggest you explain what you are trying to do.

Cheers

robert
249c7fd851c5c5ac5a1abdb756472ae1?d=identicon&s=25 Arup Rakshit (my-ruby)
on 2014-03-12 13:26
Robert Klemme wrote in post #1139595:
> On Wed, Mar 12, 2014 at 11:32 AM, Arup Rakshit <lists@ruby-forum.com>
> wrote:

>
> robert

Sorry this totally different question. As related to `refactoring` I put
here. Would you want me to post it separately ?

Sorry for creating the confusions.
E0d864d9677f3c1482a20152b7cac0e2?d=identicon&s=25 Robert Klemme (robert_k78)
on 2014-03-12 15:06
(Received via mailing list)
On Wed, Mar 12, 2014 at 1:26 PM, Arup Rakshit <lists@ruby-forum.com>
wrote:
> Robert Klemme wrote in post #1139595:

> Sorry this totally different question. As related to `refactoring` I put
> here. Would you want me to post it separately ?

Yes, that's good practice. By keeping things separate you make it
easier for people to follow, reply and find solutions later.

> Sorry for creating the confusions.

No problem.

Cheers

robert
Please log in before posting. Registration is free and takes only a minute.
Existing account

NEW: Do you have a Google/GoogleMail, Yahoo or Facebook account? No registration required!
Log in with Google account | Log in with Yahoo account | Log in with Facebook account
No account? Register here.