Topic: partial hack needs refactoring

I made this for my rails project in order to upload all files in a local directory into a photo album, but the code is messey. If you can think of any good refactoring ideas that would be awesome!

def add_mass_photos(id)
    album = Album.find(id)
    folder_path = "#{RAILS_ROOT}/public/photo/new"

    #Obviously this all needs recoding
    d = Dir.new(folder_path) 
    array = d.to_a
    array.delete(".")
    array.delete("..")

   # I guess this could be refactored too
    for photo in array
      @file = File.new(folder_path + "/" + photo )
        @temp = Photo.new(:name => album.name, :album_id => album.id, :file => @file)
        @file.close
        if !@temp.save
          puts "error"
        end
    end
  end

Re: partial hack needs refactoring

Not much to do actually. 

I'd refactor the entire folder_path into an environment file

FOLDER_PATH="#{RAILS_ROOT}/public/photo/new"

And then just use FOLDER_PATH in the method.

Where you're looping over the photos... that section doesn't need to be using instance variables (@) but could be using local variables instead.

You could pull that loop out into its own method for easier testing.  It seems to be a fairly well defined thing, after all.

But if you want it / Then you must find it
But when you have it / There'll be no need for it