Ruby 这段代码怎么去重复?

fsword · 2013年01月24日 · 最后由 fsword 回复于 2013年02月05日 · 3413 次阅读

系统中的代码,简化了是这样,感觉太丑,不知道怎么改

if concurrent?
  db_specs.map do |label, options|
    Thread.new do
      build_db label,options
    end
  end.map(&:value)
else
  db_specs.map do |label, options|
    build_db label,options
  end
end

end.map(&:value),这个眼生。是合法的么,map-reduce?

可以把只有一行代码的 block 换成 {}, 减少一些 do ... end

db_specs.map do |label, options|
    db_values = concurrent? ? build_db label,options : Thread.new { build_db label,options }
end

return concurrent? ?  db_values.map(&:value) : db_values

that's ok?

#3 楼 @xds2000 感觉没有楼主的清晰呢 ..

dry is not always a good idea.

#5 楼 @hooopo 怎么有种 omasake 的既视感。

#6 楼 @Saito yeah.it's from dhh..

Ruby 2.0 的话可以用奇技淫巧...

(concurrent? ? db_specs : db_specs.lazy).map do |label, options|
  Thread.new do
    build_db label, options
  end
end.each(&:join)

楼主的最美其实 如果要加几行,也不用再次拆开来

#3 楼 @xds2000 这个有错误,应该把 block 里的最后一行拿出来,不过还是感觉比较难看 #8 楼 @luikore 这样好像起不到我要的效果,concurrent 为 false 时返回的还是线程数组

#10 楼 @fsword 那就 map(&:value) ...

@fsword 改了。可以省 duplicate 循环

def build_dbs( db_specs )
  db_specs.map do |label, options|
    build_db label, options
  end
end

def build_dbs_concurrently( db_specs )
  db_specs.map do |label, options|
    Thread.new { build_db label, options }
  end
end

if concurrent?
  build_dbs_concurrently.map(&:value)
else
  build_dbs
end

代码更多了,重复也没有去,不过看上去是不是好一些?

有一个想法,不过好像又麻烦了

class LazyProc
  def initialize(_proc, *args)
    @proc = _proc
    @args = args
  end

  def call
    @proc.call(*@args)
  end
end

def specs_loop
  db_specs.map do |label, options|
    yield(LazyProc.new(method(:build_db).to_proc, label , options))
  end
end

if concurrent?
 specs_loop { |lp| Thread.new { lp.call } }.map(&:value)
else
 specs_loop { |lp| lp.call }
end

使用方法:

# without concurrent execution
BuildDB.new(db_specs, false).output

# with concurrent execution
BuildDB.new(db_specs, true).output

实现方法:

class BuildDB
  def initialize(db_specs, concurrent)
    @db_specs   = Collection.new(db_specs)
    @concurrent = concurrent
  end

  def output
    wrap_collection(@db_specs).format_output
  end

  private

  def wrap_collection(subject)
    Collection.new(
      subject.map do |label, options|
        wrap_execution { run label, options }
      end
    ).tap { |c| c.concurrent = @concurrent }
  end

  def wrap_execution(&block)
    if @concurrent
      Thread.new { block.call }
    else
      block.call
    end
  end

  def run(label, options)
    puts label, options
  end

  class Collection < ::Array
    attr_accessor :concurrent

    def format_output
      concurrent ? self.map(&:value) : self
    end
  end
end

dhh will probably praise the PO and give each of the following guys a finger...

这样的话似乎还好,只是性能稍微受到些影响:

class FakeThread
  def initialize
    @value = yield
  end
  attr_reader :value
end

klass = concurrent? ? Thread : FakeThread

db_specs.map do |label, options|
   klass.new do
     build_db label,options
   end
end.map(&:value)

#13 楼 @qhwa 👍 把上面的两个方法变成 private 然后扔到下面

我也觉得@qhwa 的明了些,方法的功能越单一越好。

给个 Proc 吧

if concurrent?
  db_specs.map { |label, options| Thread.new(&:build_db) }.map(&:value)
else
  db_specs.map(&:build_db)
end

?

if concurrent?
  db_specs_map(:build_db_with_thread).map(&:value)
else
  db_specs_map(:build_db)
end

def db_specs_map(method)
  db_specs.map do |label, options|
    send(method, label, options)
  end
end

def build_db_with_thread(label, options)
  Thread.new do
    build_db label, options
  end
end

在公司里提这个问题,一个哥们用 scala 给了答案:

trait Mapper {
  def apply[A, B](coll: Collection[A])(fun: A => B): B 
}

class Serial extends Mapper {
  def apply[A,B](coll: Collection[A])(fun: A => B): B = coll map { fun } 
}

class Parallel extends Mapper {
  def apply[A,B](coll: Collection[A])(fun: A => B): B = coll par map { fun } 
}

val map = if (current) parallel else serial
map (db_specs) { build_db (_, _)}

看来我的思路走歪了,其实判断 concurrent 的 if...else 是少不了的,关键是让这个结构独立出来,与具体的 array 和 block 无关,这样才能复用。

ruby 版本

def whenever args
  if concurrent?
    args.map{ |arg| Thread.new{yield arg} }.map(&:value)
  else
    args.map{|arg| yield arg}
  end
end

whenever(db_specs) do |label,options|
  build_db label, options
end

其实骨子里相当于 @fredwu 的方案进行了简化和抽象化

需要 登录 后方可回复, 如果你还没有账号请 注册新账号