Решение на Втора задача от Николай Добромиров

Обратно към всички решения

Към профила на Николай Добромиров

Резултати

  • 6 точки от тестове
  • 1 отнета точка
  • 5 точки общо
  • 12 успешни тест(а)
  • 0 неуспешни тест(а)

Код

class Song
attr_accessor :name, :artist, :genre, :subgenre, :tags
def initialize(row, info)
@tags = []
parse_row row
apply info
end
private
def parse_row(row)
fields = split_and_trim row, '.'
@name, @artist = fields[0], fields[1]
@genre, @subgenre = split_and_trim fields[2], ','
tags.push @genre
tags.push @subgenre if @subgenre
append_tags split_and_trim(fields[3], ',') if fields[3]
end
def split_and_trim(str, separator='.')
str.split(separator).map {|value| value.strip }
end
def apply(info)
append_tags info[artist] if info.has_key? artist
tags.map! {|tag| tag.downcase }
tags.uniq!
end
def append_tags( additionalTags )
additionalTags.each do |tag|
tags.push tag
end
end
end
class Criteria
def initialize(criteria)
@innerCriteria = criteria
end
def valid(song)
@validatedSong = song
valid_name && valid_artist && valid_tags && valid_filter
end
private
def valid_filter()
return true unless @innerCriteria.has_key? :filter
@innerCriteria[:filter].( @validatedSong )
end
def valid_name()
return true unless @innerCriteria.has_key? :name
check_values @innerCriteria[:name], @validatedSong.name
end
def valid_artist()
return true unless @innerCriteria.has_key? :artist
check_values @innerCriteria[:artist], @validatedSong.artist
end
def check_values(searchFor, toCompare)
if searchFor.length > 0 && searchFor[-1] == '!'
toCompare != searchFor[0..-2]
else
toCompare == searchFor
end
end
def valid_tags()
return true unless @innerCriteria.has_key? :tags
[@innerCriteria[:tags]].flatten.each do |tag|
return false unless check_collections tag, @validatedSong.tags
end
true
end
def check_collections(searchFor, searchIn)
if searchFor.length > 0 && searchFor[-1] == '!'
not ( searchIn.include? searchFor[0..-2] )
else
searchIn.include? searchFor
end
end
end
class Collection
def initialize(rawData, additionalData)
@songs = []
rawData.each_line do |row|
@songs.push Song.new( row, additionalData )
end
end
def find( criteriaParams = {} )
criteria = Criteria.new criteriaParams
result = []
@songs.each do |song|
result.push song if criteria.valid song
end
result
end
end

Лог от изпълнението

............

Finished in 0.54078 seconds
12 examples, 0 failures

История (1 версия и 2 коментара)

Николай обнови решението на 30.10.2011 17:02 (преди около 13 години)

+class Song
+ attr_accessor :name, :artist, :genre, :subgenre, :tags
+
+ def initialize(row, info)
+ @tags = []
+ parse_row row
+ apply info
+ end
+
+ private
+
+ def parse_row(row)
+ fields = split_and_trim row, '.'
+ @name, @artist = fields[0], fields[1]
+ @genre, @subgenre = split_and_trim fields[2], ','
+ tags.push @genre
+ tags.push @subgenre if @subgenre
+ append_tags split_and_trim(fields[3], ',') if fields[3]
+ end
+
+ def split_and_trim(str, separator='.')
+ str.split(separator).map {|value| value.strip }
+ end
+
+ def apply(info)
+ append_tags info[artist] if info.has_key? artist
+ tags.map! {|tag| tag.downcase }
+ tags.uniq!
+ end
+
+ def append_tags( additionalTags )
+ additionalTags.each do |tag|
+ tags.push tag
+ end
+ end
+end
+
+class Criteria
+ def initialize(criteria)
+ @innerCriteria = criteria
+ end
+
+ def valid(song)
+ @validatedSong = song
+ valid_name && valid_artist && valid_tags && valid_filter
+ end
+
+ private
+
+ def valid_filter()
+ return true unless @innerCriteria.has_key? :filter
+ @innerCriteria[:filter].( @validatedSong )
+ end
+
+ def valid_name()
+ return true unless @innerCriteria.has_key? :name
+ check_values @innerCriteria[:name], @validatedSong.name
+ end
+
+ def valid_artist()
+ return true unless @innerCriteria.has_key? :artist
+ check_values @innerCriteria[:artist], @validatedSong.artist
+ end
+
+ def check_values(searchFor, toCompare)
+ if searchFor.length > 0 && searchFor[-1] == '!'
+ toCompare != searchFor[0..-2]
+ else
+ toCompare == searchFor
+ end
+ end
+
+ def valid_tags()
+ return true unless @innerCriteria.has_key? :tags
+ [@innerCriteria[:tags]].flatten.each do |tag|
+ return false unless check_collections tag, @validatedSong.tags
+ end
+ true
+ end
+
+ def check_collections(searchFor, searchIn)
+ if searchFor.length > 0 && searchFor[-1] == '!'
+ not ( searchIn.include? searchFor[0..-2] )
+ else
+ searchIn.include? searchFor
+ end
+ end
+end
+
+class Collection
+ def initialize(rawData, additionalData)
+ @songs = []
+ rawData.each_line do |row|
+ @songs.push Song.new( row, additionalData )
+ end
+ end
+
+ def find( criteriaParams = {} )
+ criteria = Criteria.new criteriaParams
+ result = []
+ @songs.each do |song|
+ result.push song if criteria.valid song
+ end
+ result
+ end
+end
  • Не спазваш конвенциите в Ruby. Променливите се пишат със snake_case, а не с camelCase. rawData и additionalData е неприемливо. Това ти струва точка.
  • Плозваш мутиращи методи. tags = tags.uniq е много по-добре от tags.uniq!, нищо че създава още един списък. Аналогично за map!.
  • Изпускаш моменти, в които да ползваш Enumerable. Например, в Criteria#valid_tags трябваше да ползваш Enumerable#all?, вместо да правиш #each с return. В Collection#initialize трябваше да направиш @songs = rawData.lines.map вместо rawData.each_line. Във Collection#find трябваше да направиш @songs.select вместо @songs.each.
  • [@innerCriteria[:tags]].flatten е доста криптичен начин да напишеш Array(@innerCriteria[:tags]) или дори [*@innerCriteria[:tags]].
  • searchFor[0..-2] се записва като searchFor.chomp('!').
  • Предикатите в Ruby трябва да завършват на ?. Например, метода ти трябва да се казва valid?, а не valid.
  • @innerCriteria е странно име. Защо inner?
  • valid_filter е странно име. Защо "valid"? Аналогично за останалите.
  • Не си консистентен в това къде слагаш whitespace. Отделно, не спазваш конвенциите.
  • Както си говорихме в понеделник, класове ти ти имат някой предимства (няма Shotgun surgery), но имат и недостатъци:
    • Song може да се конструира само с текстов низ. Това не е хубаво ако четеш песни от някъде другаде. По-добре щеше да бъде, ако парсера на реда е изолиран от конструктора на Song.
    • Collection не прави нищо съществено.
    • Criteria ми е малко странен като клас. На практика абстрахира един хеш и малко булева логика. Щеше да е по-добре ако беше последвал препоръката от условието - проверката дали песен отговаря на критерий да бъде в Song, докато парсенето в Collection.
  • Имената ти като цяло не са добри. searchIn, searchFor и @validatedSong са трудни за разбиране.

Мерси за коментарите. Аз си ги и знаех веднага след края на лекция та в понеделник (31.10), но исках да коментираш идеята :). Преработил съм го до някаква степен (54 реда), но няма как да кача новата версия. Идеята е запазена.

Поздрави!