Ruby on Rails

On 4 July 2015 at 03:40, Federicko <fedomain@gmail.com> wrote:
> Hi Colin,
>
> I have done that but slightly differently as the 'this' variable was giving
> me an error.

Sorry it should have been 'self' not 'this' of course, as I am sure
you worked out. ruby != c++

>
> Controller didn't change. It still just calls the rankup or rankdown methods
> in the model.
> Here's the code for that:
>
> def rank
> @this_article = Article.find(params[:id])
>
> if (params[:rank] == 'up')
> @this_article.rankup
> else
> @this_article.rankdown
> end
>
> redirect_to articles_path()
> end
>
> Then in my model I have 3 methods:
>
> def rankup
>
> @affected_article = Article.find_by(ranking: ranking.to_i-1)
>
> swap_rank(@affected_article)
>
> end
>
> def rankdown
>
> @affected_article = Article.find_by(ranking: ranking.to_i+1)
>
> swap_rank(@affected_article)
> end
>
> private
>
> def swap_rank(affected_article)
>
> @current_ranking = self.ranking
>
> self.ranking = affected_article.ranking
> affected_article.ranking = @current_ranking
>
> Article.transaction do
> self.save
> affected_article.save
> end
> end
>
> This way all the code sits in one method and there is no repeated code

That looks good, though you are still not handling the error
conditions on :rank and if either of the saves fail. No doubt you
plan to sort those. Don't forget to write automated tests to check
all the methods including the failure conditions.

Why do you need ranking.to_i-1 rather than ranking-1?

Colin

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-talk@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAL%3D0gLsdmjny6UiBbk3GMCUmyOd3yyY5Bx86iKHU7mM2s62Vkg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.