我怎样才能使这个方法更简洁？

Question

当运行对 Rails 项目发臭时，我收到警告：

[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

如何最小化这种方法？

Answer 1

首先，我发现 class 和方法大小对于查找可能需要重构的代码很有用，但有时您确实需要很长的 class 或方法。并且总有一种方法可以使您的代码更短以绕过这些限制，但这可能会降低其可读性。所以我在使用静态分析工具时禁用了这种类型的检查。

此外，我不清楚为什么您会在删除故事时遇到错误，或者谁会从仅包含 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

您可以看到，在您的框架中多加注意可以减少组件中的大量噪音。

我怎样才能使这个方法更简洁？

How can I make this method more concise?

ruby

service

code-analysis

ruby-on-rails

reek