4
\$\begingroup\$

We have an action merge for two projects. In the projects we have different modules like Feed, Files, Post etc....

modules Convert
class Merge
    attr_accessor :source_ids, :destination_id, :sources, :destination

    def initialize(source_ids, destination_id)
        @source_ids = source_ids.class == Array ? source_ids : [source_ids]
        @destination_id = destination_id
        @sources = Modelname.where("id in (?)", @source_ids).all
        @destination = Modelname.where("id in (?)", @destination_id).first
    end

    def merge
        sources.each do |source|
          ActiveRecord::Base.transaction do
            Convert::Feed.new(source, destination).merge
            Convert::File.new(source, destination).merge
            Convert::Post.new(source, destination).merge
            ## There are many more modules - code comes here
          end
        end
    end
end

class Feed
    def initialize(source, destination)
        @source = source
        @destination = destination
    end

    def merge
        #Feed merge code here
    end

end

class File
    def initialize(source, destination)
        @source = source
        @destination = destination
    end

    def merge
        #File merge code here
    end

end

class Post
    def initialize(source, destination)
        @source = source
        @destination = destination
    end

    def merge
        #Post merge code here
    end

end
end


a = Convert::Merge.new(source_id, destionation_id)
a.merge

Is this correct way to handle this action ? How can I handle this in a better way ?

\$\endgroup\$
  • \$\begingroup\$ krunal, you could write @source_ids = source_ids.class == Array ? source_ids : [source_ids] as @source_ids = [*source_ids]. For example, [*[1,2,3]] => [1, 2, 3], [*2] => [2]. \$\endgroup\$ – Cary Swoveland Jun 17 '14 at 18:40
  • \$\begingroup\$ My question is not about only passing an parameters. Question is about pattern I used is correct or not ? \$\endgroup\$ – krunal shah Jun 17 '14 at 19:37
  • \$\begingroup\$ krunal, I should have said I was just making a small point, not addressing your question. \$\endgroup\$ – Cary Swoveland Jun 17 '14 at 19:54
3
\$\begingroup\$

This is nowhere near the full answer, but I just want to touch on the initialize method of Merge real quick since it was the first thing to jump out to me.

def initialize(destination_id, *source_ids)
  @source_ids = source_ids
  @destination_id = destination_id
  @sources = Modelname.where(id: @source_ids).all
  @destination = Modelname.where(id: @destination_id).first
end

This code is functionally equivalent. Using the splat operator at the end I guarantee the source ids will be in an array. I'll keep looking at the rest of the code and go from there, but that was the first thing I wanted to point out.

\$\endgroup\$
  • \$\begingroup\$ @krunal_shah is the merge code the same for File, Feed, and Post? \$\endgroup\$ – Sean May 30 '14 at 22:53
  • \$\begingroup\$ No merge code for all the modules will be different. \$\endgroup\$ – krunal shah Jun 1 '14 at 0:48

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.