Zmienne lokalne w widokach cz.2

Zakład patologii kodu
1

Zadaniem poniższego kodu jest wyświetlenie listy dostępnych rozszerzeń plików oraz rozmiaru domyślnego załącznika. Chodzi o utworzenie listy rozszerzeń domyślnego załącznika (DefaultAssetAttachment) oraz wszystkich innych załączników (OtherAssetAttachment) oraz ich rozmiarów.

Widok:

[code=ruby]<% available_files = Array.new
available_files << @asset.default_attachment.filename.gsub(/^[^.]+./,’’).upcase
@asset.other_attachments.each do |attachment|
available_files << attachment.filename.gsub(/^[^.]+./,’’).upcase
end
%>

<%= content_tag(:p, available_files.join(", “) + " (#{number_to_human_size(@asset.default_attachment.size)})” ) %>[/code]
Modele:

[code=ruby]# app/models/asset.rb
class Asset < ActiveRecord::Base

has_one :default_attachment, :class_name => ‘DefaultAssetAttachment’, :conditions => [“asset_attachments.default = ?”, true]
has_many :other_attachments, :class_name => ‘OtherAssetAttachment’, :conditions => [“asset_attachments.default = ?”, false]
end[/code]

# app/models/asset_attachment.rb class AssetAttachment < ActiveRecord::Base belongs_to :asset end

# app/models/default_asset_attachment.rb class DefaultAssetAttachment < AssetAttachment end

# app/models/other_asset_attachment.rb class OtherAssetAttachment < AssetAttachment end
Problem:

  1. Nie podoba mi się użycie zmiennej lokalnej available_files w widoku. Wygląda to tak jakby programista potrzebował pobrać dane z modeli w specyficznej formie (rozszerzenie dużymi literami oraz rozmiar domyślnego załącznika - DefaultAssetAttachment) ale zamiast dopisać potrzebne metody do modeli, które mogłyby być użyte ponownie w innych częściach aplikacji i wzbogaciły jej API użył po prostu zmiennej lokalnej aby w widoku zbudować sobie strukturę, którą dopiero będzie mógł wyświetlić
  2. Kod nie jest DRY (gsub(/^[^.]+./,’’).upcase pisane 2 razy obok siebie)
  3. Wyświetlany jest rozmiar tylko domyślnego załącznika chociaż bardziej przydatne byłoby wyświetlenie rozmiaru rownież innych załączników
2

Zaproponowany refaktoring kodu:

  1. Wyodrębnić to wyrażenie regularne jako przydatną, publiczną metodę modelu AssetAttachment
  2. Zmienną lokalną w widoku zastąpić publiczną metodą klasy Asset, która zbuduje brakującą strukturę

[code=ruby]class AssetAttachment < ActiveRecord::Base
def extension
self.filename.gsub(/^[^.]+./,’’)
end
end

class Asset < ActiveRecord::Base
def attachment_file_types_and_sizes
([default_attachment] + other_attachments).map { |x| { :extension => x.extension, :size => x.size}}
end
end[/code]
Widok po modyfikacjach:

<%= content_tag(:p, @asset.attachment_file_types_and_sizes.map {|x| "#{x[:extension].upcase} (#{number_to_human_size(x[:size])})" }.join(", ")  ) %>

Kod jest troszkę dłuższy ale zyskujemy:

  1. Możliwość użycia struktury zwróconej przez metodę attachment_file_types_and_sizes w innym widoku (zależnie od potrzeb)
  2. Kod jest bardziej przejrzysty i DRY
  3. Wyświetlamy rozmiar nie tylko domyślnego załącznika ale również innych załączników