Ruby 请教代码重构

flowerwrong · 2014年11月14日 · 最后由 emanon 回复于 2014年11月16日 · 2031 次阅读

请教如下代码如何重构,ruby 代码被我写成了 php

注意

不是简单的拆分成多个函数避免一个方法内容超过 10 行代码, 理想情况下, 大多数方法内容应该少于5行。空行不算进 LOC 里,if 嵌套里面得逻辑是层层递进的。

def create
  @msg = Msg.new(msg_params)
  article_ids = params[:article_ids]
  weixin_ids = params[:weixin_ids]

  if article_ids.length > 1
    @msg.is_multi = 1
  else
    @msg.is_multi = 0
  end

  if @msg.save
    article_ids.each do |a_id|
      MsgArticle.create({msg_id: @msg.id, article_id: a_id})
    end
    weixin_arr = []
    weixin_ids.each do |w_id|
      sleep 2
      weixin = Weixin.find w_id
      arr = [weixin.username, weixin.password]
      weixin_arr.push arr
      wx_ext = WxExt::WeiXin.new arr[0], arr[1]
      flag = wx_ext.init
      if flag
        puts "===" * 20
        articles = @msg.articles

        upload_msg_params = {
            AppMsgId: '',
            ajax: 1,
            count: article_ids.length,
            f: 'json',
            lang: 'zh_CN',
            random: rand,
            token: wx_ext.token,
            vid: ''
        }

        articles.each_with_index do |a, i|
          sleep 2
          puts "==start#{i}" * 5

          file_path = a.img.current_path
          file = File.new(file_path, 'rb')
          file_hash = wx_ext.upload_file(file, File.basename(file_path))

          if file_path["base_resp"]["ret"].to_s == '0'
            file_id = file_hash["content"]

            puts "=file_id=" * 5
            puts file_id

            article_param = {
                "author#{i}".to_sym => "#{a.author}",
                "title#{i}".to_sym => "#{a.title}",
                "sourceurl#{i}".to_sym => "#{a.sourceurl}",
                "fileid#{i}".to_sym => file_id,
                "digest#{i}".to_sym => "#{a.summary}",
                "content#{i}".to_sym => "#{a.content}",
                "show_cover_pic#{i}".to_sym => 1
            }
            upload_msg_params = upload_msg_params.merge article_param
          else
            flash[:notice] = '图文消息创建失败'
            return
          end
        end
        upload_msg_hash = wx_ext.upload_multi_msg(upload_msg_params)

        if upload_msg_hash["ret"].to_s == '0'
          puts "=upload_msg_hash_all=" * 2
          puts upload_msg_hash

          msg_list_hash = wx_ext.get_app_msg_list

          if msg_list_hash["base_resp"]["ret"].to_s == '0'
            puts "=msg_list_hash=" * 3
            puts msg_list_hash
            app_msg_id = msg_list_hash["app_msg_info"]["item"][0]["app_id"]
            puts "=app_msg_id=" * 3
            puts app_msg_id

            brondcast_msg_params = {
                ajax: 1,
                appmsgid: app_msg_id, # 图文appid
                cardlimit: 1, # 发送限制条数
                city: '',
                country: '',
                province: '',
                f: 'json',
                groupid: '-1',
                imgcode: '',
                lang: 'zh_CN',
                operation_seq: wx_ext.operation_seq, #
                random: rand,
                sex: 0,
                synctxweibo: 0,
                token: wx_ext.token,
                type: 10
            }
            brondcast_msg_hash = wx_ext.broadcast_msg(brondcast_msg_params)
            if brondcast_msg_hash["ret"].to_s == '0'
              puts "brondcast_msg_hash" * 3
              puts brondcast_msg_hash

              WeixinMsg.create({weixin_id: w_id, msg_id: @msg.id, publish_time: Time.now, is_sent: 1})

              flash[:notice] = '图文消息创建成功'
            else
              flash[:notice] = '图文消息创建失败'
              return
            end
          else
            flash[:notice] = '图文消息创建失败'
            return
          end
        else
          flash[:notice] = '图文消息创建失败'
          return
        end
      else
        puts "==" * 20
        puts "weixin.init failed"
        flash[:notice] = '图文消息创建失败'
        return
      end
    end
  end

  respond_with(@msg)
end
if @msg.save && process_one && process_two && process_three
  flash[:notice] = '图文消息创建成功'
else
  flash[:notice] = '图文消息创建失败'
end

private

def process_one
end

def process_two
end

def process_three
end

sleep 2 几个意思 💀

不要相信“一个方法不能超过几行几行”之类的话,那些都是形式上的,是鸡汤大师喜欢给新手灌输的东西...

你要关心的是职责是否单一、是否低耦合、高聚合这些本质的东西。

当然,你能写出这么长的方法,我也没什么想说的了...

写成这样都没有队友骂你的话,你还要继续这样写啊!

还有……建议把 if/else 反过来,尽可能做成 raise if xxx 这样的结构,看着应该会比较清晰些。

“一个方法不超过几行”只是结果,不是目的,更准确点说应该是“不会超过几行”,而不是“不能超过几行”。Ruby 代码仔细写的话, 大多数 方法控制在 10 行内应该是不难做到的——但这不是目的,在能力不到的情况完全可以不这么要求自己。

我们的整体环境好像都比较喜欢把结果当目的来教给别人,一方面可能是这个环境比较急功近利,另一方面这种教法容易让人迷糊,给人高大上的感觉吧。当然也不排除新人自己理解错误,自己把结果当作目的,努力错了方向。

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