On 4 July 2015 at 03:40, Federicko <[email protected]> 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 [email protected]. To post to this group, send email to [email protected]. 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.
