一個在面試時被問的一個問題(一段ruby on Rails的code)


#1

大家好 我是一個在美國的留學生昨天在一個面試中被問了兩個問題我沒有非常懂所以想請教一下要怎麼回答下面這個問題

class ScenariosController < ApplicationController
  include ScenarioCopier
  before_action :set_scenario, only: [:show, :copy, :edit, :tree, :update, :destroy]
  respond_to :java
  # GET /scenarios
  def index
    authorize! :index, Scenario
    render json: Scenario.where(:locked => false, :university => current_user.university)
  end
  # GET /scenarios/1
  def show
    authorize! :read, @scenario
    render json: @scenario
  end
  # GET /scenarios/1/edit
  def edit
  end
  def tree
    authorize! :read, @scenario
    render json: @scenario, choices: @scenario.complete_choices, serializer: ScenarioTreeSerializer
  end
  # POST /scenarios
  def create
    authorize! :create, Scenario
    scenario = Scenario.new(scenario_params)
    scenario.university = current_user.university
    scenario.archived = false
    scenario.locked = false
    Scenario.transaction do
      begin
        save_scenario_with_root_node(scenario)
      rescue ActiveRecord::RecordInvalid => invalid
        render json: invalid.record.errors, status: :unprocessable_entity and return
      end
      render json: scenario, status: :created
    end
  end
  # POST /scenarios/1/copy
  def copy
    authorize! :copy, @scenario
    unless scenario_params[:name]
      render json: {error: 'Must provide a new scenario name.'}, status: :unprocessable_entity and return
    end
    scenario_copy = copy_scenario(@scenario, {:copy_name => scenario_params[:name], :description => scenario_params[:description]}); return if performed?
    render json: scenario_copy, status: :created
  end
  # PATCH/PUT /scenarios/1
  def update
    authorize! :update, @scenario
    if @scenario.locked
      render json: {error: 'Cannot update a locked scenario.'}, status: :bad_request and return
    end
    if @scenario.update(scenario_params)
      head :no_content
    else
      render json: @scenario.errors, status: :unprocessable_entity
    end
  end
  private
    def set_scenario
      @scenario = Scenario.find(params[:id])
    end
    def scenario_params
      params.require(:scenario).permit(:name, :description, :archived, :root_node_id)
    end
    def save_scenario_with_root_node(scenario)
      scenario.save!
      root_node = initialize_root_node(scenario)
      scenario.update!(root_node_id: root_node.id)
    end
    def initialize_root_node(scenario)
      node = Node.create!(:scenario_id => scenario.id)
      node.update!(is_start: false, label: 1)
      node
    end
end

還有一段話是這段code的背景

“This segment of syntax implements aspects of user interface in an existing software. It binds the internal representation(s) of information that enable(s) the processing of Scenarios and the way the user interacts with those Scenarios.”

這是面試那裡給我的code 面試官說compromise這些code的modularity還有分析這些code哪裡缺少了cohesion. 我想請教一下有沒有人能看得懂的能講解一下這兩個問題應該要怎麼回答?


#2

test20170225.zip (2.5 KB)

原文我改了排版,這題我改了一票東西…不過部分不瞭解題意所以放棄|||,這題不好寫 X"D

首先全 json 輸出,edit 頁面不需要所以移除,增加 json column filter,還有一些邏輯有問題的地方唄,至於其他的我大部分都有寫在註解內了,有問題再提出

這題我其實會寫成 Scenario 本身就是 tree 型態,可以少掉 Node 這個看起來非常多餘的 model,不過怕改太多就這樣子就好了,除了題意不明的地方改成這樣應該不太能挑剃了,當然 code 本身沒驗證是否正確,然而你可以看我所修改的東西去推敲為何要這樣修改才是

其他人也可以嘗試寫看看然後對我的答案唄,當然我的答案也不一定是對的就是 X"DD


#3

太感謝你了!!


#4

這題在考的是有些方法不太適合放在 controller:
save_scenario_with_root_node(scenario)
initialize_root_node(scenario)
這兩個方法放在 model 會比較好一點,有兩個理由:

  1. 避免重複(DRY)
  2. 職責:兩個方法職責應屬於 Model,甚至是獨立成 Module 來管理 Model 延伸的資料結構

1.避免重複

舉個例子:如果你前後台都要管理 Scenario
假設前台後台controller 分別是 scenario_controller.rb 與 admin/ scenario_controller.rb
有可能兩者都需要用到 save_scenario_with_root_node(scenario)、initialize_root_node(scenario) 這兩個方法
要避免重複勢必要萃取出來。

###2. 職責
決定要萃取出來後就要開始思考放在哪邊比較好,
放在 controller 的 private method 是個不好的選擇,
一來每個 action 做的事情可能是好幾件事情的組合
所以一個簡單的分類是如果是某個 「model 做的動作」就放在哪個 Model 底下。
如果 Model 的方法過多,可以依照動作的類型模組化。
如果將相對瑣碎的步驟整理到每個歸屬的 model 甚至模組化自然就提高了內聚力。


#5

!!謝謝這位大大!


#6

你每個都這樣做你的 model 會肥到翻唄 … 所以有所取捨就好了,寫在 action 也行,如果是真的一次性的話


#7

看了一下原句

Please identify the segment(s) of code that lack cohesion and compromise the modularity of the code.

後面的 compromise 那段應該是問說哪段code應該被模組化而沒有