ruby-on-rails – 如何使这种方法更简洁？

2019年7月20日 152次阅读

在Rails项目上运行reek时我收到警告：

[36]:ArborReloaded::UserStoryService#destroy_stories has approx 8 statements (TooManyStatements)

这是方法：

def destroy_stories(project_id, user_stories)
  errors = []
  @project = Project.find(project_id)
  user_stories.each do |current_user_story_id|
    unless @project.user_stories.find(current_user_story_id).destroy
      errors.push("Error destroying user_story: #{current_user_story_id}")
    end
  end
  if errors.compact.length == 0
    @common_response.success = true
  else
    @common_response.success = false
    @common_response.errors = errors
  end
  @common_response
end

如何最小化这种方法？

最佳答案 首先,我发现类和方法大小对于查找可能需要重构的代码很有用,但有时您确实需要一个长类或方法.并且始终有一种方法可以缩短您的代码以克服这些限制,但这可能会降低您的可读性.所以我在使用静态分析工具时禁用了那种类型的检查.

此外,我不清楚为什么您在删除故事时会出现错误,或者从包含ID的错误消息中获益,而不会发生错误.

也就是说,我会像这样编写这种方法,以减少显式的本地状态并更好地区分问题：

def destroy_stories(project_id, story_ids)
  project = Project.find(project_id) # I don't see a need for an instance variable
  errors = story_ids.
    select { |story_id| !project.user_stories.find(story_id).destroy }.
    map { |story_id| "Error destroying user_story: #{story_id}" }
  respond errors
end

# Lots of services probably need to do this, so it can go in a superclass.
# Even better, move it to @common_response's class.
def respond(errors)
  # It would be best to move this behavior to @common_response.
  @common_response.success = errors.any?
  # Hopefully this works even when errors == []. If not, fix your framework.
  @common_response.errors = errors
  @common_response
end

您可以看到在您的框架中如何小心处理可以在组件中节省大量噪音.

点赞