Przekazywanie paramsów i poruszanie się po linkach

Zakład patologii kodu
1

Hej,
Właśnie zacząłem pisać moją pierwszą aplikację w railsach, w sumie to, to co chciałem działa, ale mam jakieś dziwne wrażenie, że można to wszystko zrobić dużo ładniej i przejrzyściej.

O co chodzi.
Mam dwa zestawy linków:

  1. Rodzaj wpisów: Kategoria1, Kategoria2, Kategoria3 - odpowiednio ?type=0,1,2 i brak ?type - wszystkie
  2. Sortowanie: Popularne, Komentowane, Najlepsze - odpowiednio ?order nil,1,2,
  3. link Next z willpaginate

Powinno to działać standardowo, czyli klikam Kategoria1 i ten ?type=0 jest cały czas przy linkach do sortowania, tak samo z Kategoria2 i 3.

Będę wdzięczny za wszelkie konstruktywne zjebki (na razie to pierwszy kontroler i pierwsza akcja + mega prosty model bez żadnych walidacji:)

Tutaj, wydaje mi się, że to moje generowanie t, żeby później zrobić where("#{t}") jest jakoś na około, pewnie da się dużo prościej takie zapytanie sklecić.

class EntryController < ApplicationController def index #params[:order] - brak - popularne (domyslne), 1 - komentowane, 2 - najlepsze #params[:type] - 0 kategoria1, 1 - kategoria2, 2 - kategoria3 #buduje t = wygenerowany fragment pod where 'entry_type = ...' !params[:type]? t = "1==1" : t = "entry_type="+params[:type] if !params.has_key?(:order) @entries=Entry.where("#{t}").order("rank DESC").page(params[:page]).per_page(20) elsif params[:order] == "1" @entries=Entry.where("#{t}").order("comments_count DESC").page(params[:page]).per_page(20) elsif params[:order] == "2" @entries=Entry.where("#{t}").order("points_count DESC").page(params[:page]).per_page(20) end end end
A we view mam taki kod (dodaje class=“active” dla aktualnie wybranego linku i zamienia go w zwykly test) - jakoś mi ten html_safe śmierdzi, ale nie chciałem rozbijać tak krótkiego kodu na jakieś większe if elsy.

<nav> <ul class="navbar"> <li><%= !params[:order] ? "<span class=\"active\">Popularne</span>".html_safe : link_to("Popularne",:type=>params[:type])%></li> <li><%= params[:order] == "1" ? "<span class=\"active\">Nowe</span>".html_safe : link_to("Nowe",{:order=>1,:type=>params[:type]})%></li> <li><%= params[:order] == "2" ? "<span class=\"active\">Najlepsze</span>".html_safe : link_to("Najlepsze",{:order=>2,:type=>params[:type]})%></li> </ul> </nav>

2

Tak na szybko, z t możesz zrobić tak:

t = "entry_type=" + params[:type] unless params[:type].empty?

Jak nie będzie, to dostaniesz nila, i później:

o = case params[:order] when "1" then "comments_count" when "2" then "points_count" else "rank" end o += " DESC" @entries = Entry.where(t).order(o).page(params[:page]).per_page(20) unless params[:order].empty?
Rzecz w tym, że to i tak niepełen refactor - ja bym wyjął jeszcze to mapowanie ordera poza metodę kontrolera.

3

[code=ruby]class EntryController < ApplicationController
def index
@entries = Entry.search(params)
end
end

class Entry < ActiveRecord::Base
def self.search(params)
t, o = search_type(params[:type]), search_order(params[:order])
scope = self

scope = scope.where(t) if t
scope.order(o).page(params[:page]).per_page(20)

end

private

def search_order(order)
return ‘comments_count DESC’ if order == ‘1’
return ‘points_count DESC’ if order == ‘2’

'rank DESC'

end

def search_type(type)
return nil unless type.present?

"entry_type = #{type}"

end
end[/code]

4

Wielkie dzięki, pięknie to wygląda.

ps. tam zamiast def search_type i search_order powinno być self.search_type i self.search_order, nie kapuję tylko dlaczego - będę wdzięczny za wytłumaczenie, ewentualnie jakiś link.

5

ahh, słusznie, literówka. #search to metoda klasowa (wywoływana na klasie a nie instancji klasy), więc może wołać tylko inne metody klasowe, #search_type i #search_order były tutaj metodami instancji.

6

Fajnie, że się ta literówka pojawiła, bo dowiedziałem się czegoś nowego :slight_smile:

7

[code=ruby] def search_order(order)
return ‘comments_count DESC’ if order == ‘1’
return ‘points_count DESC’ if order == ‘2’

'rank DESC'

end[/code]
Polecam w takich przypadkach zamiast ifów zrobić np. hash

[code=ruby]search_order[order]

def search_order
{ ‘1’ => ‘comments_count DESC’, ‘2’ => ‘points_count DESC’, nil => ‘rank DESC’ }
end[/code]
Jak ‘rank DESC’ ma być użyty dla każdego innego przypadku, a nie tylko dla nil to możesz użyć domyślnej wartości hasha.

8

W Twoim rozwiązaniu Staszku nie podoba mi się to, że trzeba przekazać nil, żeby dobrać się do domyślnego sortowania. Można to naprawić tak:

[code=ruby]SEARCH_ORDER = {
‘1’ => ‘comments_count DESC’,
‘2’ => ‘points_count DESC’
}

def search_order(order)
SEARCH_ORDER[order] || ‘rank DESC’
end[/code]
No dobra, a na serio, co to za odgrzewanie tematów? Staszkowi ukradli konto i się kolejny SEO spammer szykuje?

EDIT: case/when/else już był

9

chce dobić do 300 postów i zostać moderatorem :wink:

10

Można też użyć hasha z domyślną wartością jak pisałem wcześniej.

Nawet nie zwróciłem na to uwagi. Już więcej tak nie zrobię, bo teraz pełno nowych moderatorów na forum :slight_smile: