Forum: Ruby on Rails overly complex controller?

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Ac25a42f3e336878aa9aca1b88118dbb?d=identicon&s=25 tayknight (Guest)
on 2007-06-19 22:13
(Received via mailing list)
Hi all. A simple app. Albums have songs. Songs have artists. Models
look like:

class Album < ActiveRecord::Base
  has_many :albumappearances
  has_many :songs, :through => :albumappearances

  validates_length_of :description, :minimum => 4
end

class Albumappearance < ActiveRecord::Base
  belongs_to :album
  belongs_to :song
end

class Song < ActiveRecord::Base
  has_many :albumappearances
  has_many :albums, :through => :albumappearances
  belongs_to :artist

  validates_length_of :name, :minimum => 4
end

class Artist < ActiveRecord::Base
  has_many :songs
  validates_length_of :name, :minimum => 6
end

# --------------
At album create or update, a user can enter a totally new artist/song
combination. If the artist or song don't exist, they need to get
created.

So, for example, in my controller, i know I can have something like:
def update

  # figure out which album we're talking about.
  @album = Album.find(params[:id])

  # set the album description the user entered
  @album.description = params[:album][:description]

  # we'll need an artist.id later to associate with the song.
  # so start with artist.
  @artist = Artist.find(params[:artist][:name]) rescue nil

  # if no artist was found, we need to create one.
  if @artist == nil
    @artist = Artist.new(:name => params[:artist][:name])
    # try to save the artist. errors will be captured.
    @artist.save
  end

  @song = Song.find(params[:song][:name]) rescue nil

  # if we couldn't find a song, we need to create one.
  if @song == nil
    @song = Song.new(:name => params[:song][:name])

    # see if there are any validation errors with the song
    @song.valid?

    # if there isn't an artist and there are errors associated with
the artist...
    if @artist.errors && @artist.id = nil
      # don't try to save the song since it won't have an ID to use
for artist
      # but do try to validate the song so we can tell the user about
the problem
      @song.valid?
    else
      # try to save the song
      @artist.songs << @song
    end
  end

  # now, work on the album.
  if @song.errors || @artist.errors
    # if there were errorsdon't try to save the album, but validate to
show the user errors
    @album.valid?
    render :action => 'edit'
    # exit
  else
    # otherwise save the album.
    if @album.save
      flash[:notice] = 'album was successfully updated.'
      redirect_to :action => 'show', :id => @album
    else
      render :action => 'edit'
    end
  end
end

# -----------------

Now, this seems to work, but it seems awfully verbose and un rails-
like. Should some of this get moved to the models? Am I missing
something obvious?
This topic is locked and can not be replied to.