patternrubyrailsMinor
Checkboxes array to delete images
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
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:
view/_form.html.erb
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
endview/_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
Second,
Here is how I suggest to rewrite 'deletion' part:
Other parts (for
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.
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
endOther 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
endContext
StackExchange Code Review Q#118942, answer score: 3
Revisions (0)
No revisions yet.