How can I make this method more concise?

Question

I get a warning when starting reek in a Rails project:

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

Here's the method:

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

How can this method be minimized?

Answer 1

-, , , , . , , . .

, , , , , .

, , , :

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 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

, .