4

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

[36]:ArborReloaded::UserStoryService#destroy_stories 有大约 8 个语句 (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

如何最小化这种方法？

4

1 回答 1

2

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

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

你可以看到在你的框架中多加小心可以在你的组件中节省大量的噪音。

于 2016-01-16T02:06:36.593 回答