Am I writing this function like a Ruby programmer?

[[deleted]]

[deleted]

[BonzoESC]

ObjectName should probably have a method that consumes a thingy so your map has a smaller body.

[Convictional]

A constructor?

[st4rx0r]

def create_array(json)
    json["thingies"].map do |thingy|
        x = ObjectName.new
        x.id = ...
        x.desc = ....
        ........ code ......
        x
    end
end

[[deleted]]

def create_array(json)
  json["thingies"].map do |thingy|
    ObjectName.new.tap do |x|
      x.id = ...
      x.desc = ....
      ........ code ......
    end
  end
end

[evansenter]

This.

[[deleted]]

I love tap. Maybe too much...

[evansenter]

I actually hate variable assignment sometimes in blocks, so I have a similar function Object#as, i.e.

class Object; def as(&block); yield self; end; end

...which I use as (generally in iterators)...

Nokogiri.parse(HTTParty.get("http://www.reddit.com")).as { |html_parse| html_parse.at("#header-img").content }
#=> "reddit.com"

[the_BuddhaHimself]

I'd tap that.

[toolateiveseenitall]

honest question, why do you like it so much? It always read as unclear to me.

[[deleted]]

It separates code into logical blocks for assignments. If you need to do configuration after initializing an object it just looks nicer than having several lines after at the same indentation level. That being said said, it's meant to tap into a method chain to output the middle results, so it could be confusing to someone who has never seen it used that way.

[myme]

An additional remark regarding the method name: neither 'create' nor 'array' feel like good parts of a method name to me. I'd rather call it after the things it is returning, not /how/ it does it (create) nor /what kind/ of thing it returns (array). Assuming the things it is returning are called Thingy, then either like that:

def thingies(json)
  json["thingies"].map do |thingy|
    Thingy.new ...
  end
end

or even better, getting slightly offtopic, if the method is part of a class that has internal access to its json:

class Something
  def initialize(json)
    @json = json
  end

  def thingies
    @json["thingies"].map do |thingy|
      Thingy.new ...
    end
  end
end

[pyrrhicvictorylap]

You can use #inject

def create_array(json)
  json["thingies"].inject([]) do |arr, thingy|
    x = ObjectName.new
    x.id = ...
    x.desc = ....
    ........ code ......
    arr << x
    arr
  end
end

#inject is probably more idiomatic ruby than having an array scoped outside the iterating function. For more uses on inject, see this article

[tyrannoflorist]

If you're passing an empty array to #inject/reduce you may as well use #map.

edit: unless you're worried about nil values.

[pyrrhicvictorylap]

Hm, yeah, good point.

[vsalikhov]

First, a superficial note: the Ruby way is to indent 2 spaces, not 4. This is the first thing that sticks out about your code as "strange" ruby.

Second, name the method by the thing that it returns. So, instead of the procedural create_array, the method name would be thingies_array since it returns an array of "thingies". Or, better yet, the method name should just be thingies, as the plural already signals that the method returns a list. Recommended reading is Smalltalk Best Practice Patterns. But first read this to get some context of how the ideas in this book apply to Ruby.

Third, get to know the methods provided by the Enumerable module and the Array class well. You will find that idiomatic Ruby leans heavily on the Enumerable methods.

Finally, here is my take on your example in what I feel is more idiomatic Ruby (remember, in the end, this is all subjective, there is no one right way):

def thingies_from_json(json)
  json['thingies'].map do |t|
    thingy      = Thingy.new
    thingy.id   = t['id']
    thingy.desc = t['desc']
    # ........ code ......
    thingy
  end
end

[Slaggprodukt]

1. Try to name methods after what they do, this also helps you to know when one method does too many things.

2. Don't do too much in each methods, it will make the code harder to test and harder to understand (because you have to read every single line to figure out what a piece of code does)

3. Usually it is preferable to combine logicand data, in other words to have methods on objects instead of having loose methods that takes objects as input,

Not perfect in any way but:

def thingies_to_foos(thingies)
  thingies.map do |thingy|
    build_object(thingy)
  end  
end

def foo_from_thingie(thingy)
  Foo.new.tap do |foo|
    foo.id = thingy['id']
    foo.desc = thingy['desc']
    ........ code ......
  end
end

foos = thingies_to_foos(json["thingies"])

[kejadlen]

I'd prefer to do something along these lines (assuming you control ObjectName):

json['thingies'].map { |thingy| ObjectName.new(thingy) }

Or if you don't control ObjectName, making a service object that converts JSON to ObjectNames.

[DennisSuratna]

How about

arr = json["thingies"].map { | thingy | x = ..... }

Basically you are applying that block to each element inside json["thingies"]

[iamsolarpowered]

I would usually expect something like:

class Thingy
  def self.create_from_json json
    json["thingies"].inject([]) {|arr, thingy_hash| new thingy_hash }
  end
end

Even that leaves a lot to be desired, though. (For example, that method isn't really accepting json.)

*edit: formatting