HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Checkboxes array to delete images

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arrayimagesdeletecheckboxes

Problem

Basically I'm displaying an array of images on a page with checkboxes. The idea is when a user is editing a post and the checkbox is checked, the image will get deleted. Each image also has an order_id and description.

I'm pretty new to Ruby and programming and I'm looking for a way to simplify the following code.

Codeclimate is saying:


Cyclomatic complexity for update is too high

lessons_controller.rb:

def update

@lesson.attachments.each.with_index do |attachment, index|

  unless params[:delete_array].nil?
    if params[:delete_array][index].to_i == attachment.id && params[:delete_array][index].to_i != 0
      puts "DELETE: #{attachment.attachment_file_name}"
      attachment.destroy
    end
  end

  unless attachment.nil?
    unless params[:order_array].nil?
      if params[:order_array][index].to_i != attachment.order
        attachment.order = params[:order_array][index]
        puts "ORDER: #{attachment.order} = #{params[:order_array][index]}"
        @save = true
      end
    end

    unless params[:description_array].nil?
      if params[:description_array][index] != attachment.description
        attachment.description = params[:description_array][index]
        puts "DESCR: #{attachment.description} = #{params[:description_array][index]}"
        @save = true
      end
    end
  end
end

if @save == true
  @lesson.attachments.each(&:save)
end

  respond_to do |format|
  if @lesson.update(lesson_params)
    format.html { redirect_to @lesson, notice: 'Lesson was successfully updated.' }
    format.json { render :show, status: :ok, location: @lesson }
  else
    format.html { render :edit }
    format.json { render json: @lesson.errors, status: :unprocessable_entity }
  end
end
end


view/_form.html.erb


  
    
    

      
        
          

           Delete

          
          

        
      
    
      No attachments found.
    
  

Solution

I try to start from part dealing with removing of checked attachments.

First, I can't find purpose of puts lines. Only if it used for debugging in development.

Second, @lesson.attachments.each.with_index - every time updating single attachment, you need to go through all existing records. I think, that is what 'Codeclimate' "says". So, that structure must be simplified.

Here is how I suggest to rewrite 'deletion' part:

def update

  params[:delete_array].try :each do |id|
    @lesson.attachments.find(id).destroy
  end

# other code

end


Other parts (for params[:order_array] and params[:description_array]) can be refactored in the same way. I hope, you've got the idea.

If you need more detailed explanation of above code or need help with other parts - let me know and I'll explain it in details.

Code Snippets

def update

  params[:delete_array].try :each do |id|
    @lesson.attachments.find(id).destroy
  end

# other code

end

Context

StackExchange Code Review Q#118942, answer score: 3

Revisions (0)

No revisions yet.