# Overly complex controller?

**URL:** <https://www.ruby-forum.com/t/overly-complex-controller/225953>\
**Category:** Rails\
**Created:** [June 19, 2007, 10:13pm UTC](https://www.ruby-forum.com/t/overly-complex-controller/225953 "2007-06-19T22:13:50Z")\
**Posts on this page:** 1\
**Page:** 1

<div class="post-metadata">

**Author:** ![tayknight](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/da6949/32.png) [@tayknight](https://www.ruby-forum.com/u/tayknight)\
**Post date:** [June 19, 2007, 10:13pm UTC](https://www.ruby-forum.com/t/overly-complex-controller/225953/1 "2007-06-19T22:13:50Z")

</div>

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?