改進這段代碼。作品，但看起來可疑

我希望任何人幫助改善這段代碼。它目前的作品，但總是看起來懷疑我。改進這段代碼。作品，但看起來可疑

def sms 
    @message = Message.new 
    decoded_to = CGI.unescape(params[:to]) 
    #@message.to = decoded_to.gsub(/[^\d]/,"") 
    @message.to = CGI.unescape(params[:to]).strip.gsub("+","").gsub(/\s+/, "") 
    @message.from = CGI.unescape(params[:from]) 
    @message.message = CGI.unescape(params[:message]).strip 
    @message.user_id = current_user.id 
    @message.status = 'Queued' 
    if @message.save 
     MessageWorker.perform_async(@message.id, [""], current_user.id) 
     render json: {status: "Success"} 
    else 
     render json: {status: "Failed" } 
    end 
end

這是一個與其他控制器使用的消息模型交互的API控制器。它嚴格用於API交互。

來源

2013-10-26 acacia

這是你需要編寫測試，以消除你心中的不確定性和焦慮。而且只是推出這樣的代碼不會幫助。解釋請求類型，完成的過程以及您期待的響應。 –

將所有這些垃圾移到「Message」類的實例化中。這樣的事情：[清理島＃5]（http://pastebin.com/ih3HVupb） –

我最好做一種FormObject來處理所有參數處理，然後將它們傳遞給Message＃new –

分離出PARAM清洗邏輯：

def sms 
    @message = Message.new(clean_params.merge(user: current_user, status: 'Queued')) 
    if @message.save 
     MessageWorker.perform_async(@message.id, [""], current_user.id) 
     render json: {status: "Success"} 
    else 
     render json: {status: "Failed" } 
    end 
end 

private 

def clean_params 
    cleaned_params = {} 
    %i(to from message).each { |key| cleaned_params[key] = CGI.unescape(params[key]) } 
    cleaned_params[:to].strip!.gsub!("+","").gsub!(/\s+/, "") 
    cleaned_params[:message].strip! 
    cleaned_params 
end

未經檢驗的，我希望沒有錯別字了進去。）

來源

2013-10-26 10:42:53

修改params散列並不是一個好主意。看到這個http://rails-bestpractices.com/posts/807-don-t-modify-the-params-hash –

@AndreyKryachkov - 對，我更新了代碼。 –

改進這段代碼。作品，但看起來可疑

回答

相關問題