How to clean up this code?



I’ve got 2 tables: products and images. Product has_many images, image
belongs_to product. In images table i store product_id and name of the
image (not the image itself).

I’ve created a single form, where user can add any number of images for
product he/she is creating (new fields are created using javascript).
The images that are uploaded are saved to public/images folder.

I change names of the uploaded images to: + ‘-’ + number of the field from which the file was uploaded
so i.e. adding an image in the 4th field to the product with id 12 will
create the image with name ‘12-4.jpg’. Unfotunately this leads to the
problem with adding images during editing the product.

While editing the product user can delete any of images that belongs to
that product and add new ones, but i’ve got problems with naming files
added this way.

As i’m ror newbie, the code starts to become one big mess. Here it is:

— product controller ----
def new
@product =
@file_fields_no = 4
@file_fields_no.times do @product.images << end

def create
@product =[:property])
uploaded_image_number = 0 # number of the uploaded file
save_image_number = 0 # number of the file that will be saved
on the disk
# for every image
params[:images].length.times do
# check if it was sent
!= 0)
# don’t know yet how to get file extension :slight_smile:
ext = ‘jpg’
# create new filename
image_filename =
# add new name to the database
@product.images << => image_filename)
# save file on the disk"#{RAILS_ROOT}/public/images/#{image_filename}",
“wb”) do |f|
save_image_number += 1
uploaded_image_number += 1
flash[:notice] = ‘Product was successfully created.’
redirect_to :action => ‘list’
render :action => ‘new’

----- form.rhtml partial (just the part of it) -----
<%= javascript_tag “var image_field_number=#{@file_fields_no}” %>
<% if(controller.action_name == ‘new’) %>
<% @product.images.each_with_index do |image, i| %>

<% end %>
<% elsif(controller.action_name == ‘edit’) %>
<% @product.images.each do |image|%>

<%= image_tag(image.image_url) %>
<%= link_to_remote(‘delete’,
:complete =>
:url => {:controller => ‘image’, :action =>
‘destroy’, :id =>},
:confirm => “Are you sure?”) %>

<% end %>
<% end %>
<%=link_to_function ‘add image’,
‘add_image_upload_field(image_field_number); image_field_number++;’, :id
=> “add_image” %>

— add_image_upload_field js function ----------
function add_image_upload_field(id) {
var image_field = ‘
new Insertion.Before(“add_image”, image_field);

I’ve also added before_delete method to image model to delete the file
when the record is deleted from the database.

I didn’t present here ‘update’ action for the product controller,
because i don’t know how to create names for new pictures. The method
with input field number doesn’t work anymore, because the first input
file field in edit form will have number 0 and there can be already an
image with this number (‘xx-0.jpg’).

Is there an easy way to figure out what is the name of the last image
for a given product and create new one with a higher value?
I could start with 0, create filename for this number (‘xx-0.jpg’), try
to open this file and if it already exists increment the value
(‘xx-1.jpg’) and try again. Do you have any other (simpler) ideas?

How to clean up this code? Could parts of it be moved to helpers or

Thanks in advance.


I’ve done adding images via edit form by checking if file exists and if
not creating it, if it does trying next name and so on.

But i’d be very grateful if anyone could give me (even very general)
suggestions how to clean up my code.


Check out file_column plugin… that could clean up a lot of your code

----- Original Message -----
From: “g0nzo” removed_email_address@domain.invalid
To: removed_email_address@domain.invalid
Sent: Sunday, March 05, 2006 12:36 PM
Subject: [Rails] Re: How to clean up this code?