Николай обнови решението на 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 реда), но няма как да кача новата версия. Идеята е запазена.
Поздрави!