1 2 3 4 5
def upgrade_and_restart @myvocabulary.upgrade nextmyvocabulary = Myvocabulary.draw(current_user.id, params[:position], @myvocabulary.vocabulary.lesson.proficiency.id) redirect_to myvocabulary_url(nextmyvocabulary, :position => params[:position]) end
Refactorings
No refactoring yet !
Johannes
June 22, 2009, June 22, 2009 23:21, permalink
Something like this maybe?
1 2 3 4 5 6 7 8 9 10
def upgrade_and_restart @myvocabulary.upgrade nextmyvocabulary = Myvocabulary.draw( current_user.id, params[:position], @myvocabulary.vocabulary.lesson.proficiency.id ) redirect_to myvocabulary_url(nextmyvocabulary, :position => params[:position]) end
Martin Plöger
June 23, 2009, June 23, 2009 08:35, permalink
You should not chain methods that long. (http://en.wikipedia.org/wiki/Law_of_Demeter)
Rails Url-Helpers generally accept a whole object instead of its id. So you can get rid of extracting their id-attribute. Maybe you could do the same?
1 2
#OLD: @myvocabulary.vocabulary.lesson.proficiency.id @myvocabulary.lession_proficiency #New: Introduce a method that delegates internally to the proficiency of the lesson
dive.myopenid.com
June 23, 2009, June 23, 2009 12:23, permalink
@Johannes, thx, but I think the cool thing about not breaking it up is that a grep or cmd+shift+f over the whole project is easier to read
@Martin, ok, there's some hints ... like eg see if I really have to pass current_user.id or to have the helper check what kindof object it got and then extract what it needs ...
more hints?
Martin Plöger
June 23, 2009, June 23, 2009 15:04, permalink
Why do you pass params[:position] to the Myvocabulary-Object and to the myvocabulary_url? Can't you extract the parameter later on in the view from the Myvocabulary-Object?
Adam
June 23, 2009, June 23, 2009 15:27, permalink
I feel that you will want something along these lines, but it is hard to say for sure without seeing more code. What do your models look like?
1 2 3
def upgrade_and_restart redirect_to @vocabulary.upgrade_and_restart(params[:position]) end
I wonder how you make a line like this more readable.
I mean it is quite readable, only it is a bit long, and lines like this break in the editor, so it's actually not very readable ... do you worry about things like this? how would you make this more beautiful?
(actually I already made a useless variable definition "nextmyvocabulary", just to shorten the redirect_to)