Елена обнови решението на 30.10.2011 21:15 (преди около 13 години)
+class Song
+ attr_accessor :name, :artist, :genre, :subgenre, :tags
+
+ def initialize(name, artist, genre, subgenre, tags)
+ @name = name
+ @artist = artist
+ @genre = genre
+ @subgenre = subgenre
+ @tags = tags
+ end
+
+ def matches?(criteria)
+ if criteria == {}
+ return true
+ end
+ name = criteria.fetch(:name, "")
+ artist = criteria.fetch(:artist, "")
+ tags = criteria.fetch(:tags, [])
+ verify_value(@name, name) and
+ verify_value(@artist, artist) and
+ verify_tags(@tags, tags)
+ end
+
+ private
+
+ def verify_value(value, expected)
+ if expected == "" or value == expected
+ return true
+ end
+ false
+ end
+
+ def verify_tags(tags, expected)
+ if expected.kind_of?(String)
+ expected = Array(expected)
+ end
+ expected.each do |tag|
+ if tag.end_with?("!") and tags.include?(tag.chop)
+ return false
+ elsif not tag.end_with?("!") and not tags.include?(tag)
+ return false
+ end
+ end
+ true
+ end
+end
+
+class Collection
+ ArtistTags = {
+ 'John Coltrane' => %w[saxophone],
+ 'Bach' => %w[piano polyphony],
+ }
+
+ def initialize(songs_as_string, artist_tags = ArtistTags)
+ @collection = parse_songs(songs_as_string, artist_tags)
+ end
+
+ def find(criteria)
+ if criteria == {}
+ return @collection
+ end
+ matched = @collection.select {|song| song.matches? criteria}
+ p matched
+ block = criteria[:filter]
+ if block
+ block_arr = @collection.select {|song| block.(song)}
+ p block_arr
+ return matched & block_arr
+ end
+ matched
+ end
+
+ private
+
+ def parse_songs(songs_as_string, artist_tags)
+ songs_strings = songs_as_string.lines
+ songs_strings = songs_strings.map {|line| line.strip }
+ songs = songs_strings.map {|song| song_string_to_object(song)}
+ songs.each {|song| song.tags += artist_tags.fetch(song.artist, [])}
+ end
+
+ def song_string_to_object(song)
+ song_arr = song.split(%r{\.\s*})
+ name = song_arr[0].strip
+ artist = song_arr[1].strip
+ genres = song_arr[2]
+ tags = song_arr[3]
+ construct_song(name, artist, genres, tags)
+ end
+
+ def construct_song(song, artist, genres, tags)
+ separator = %r{,\s*}
+ genre = genres.split(separator)[0].strip
+ subgenre = genres.split(separator)[1]
+ tags = tags || ""
+ tags = tags.split(separator) + Array(genre.downcase)
+ if subgenre
+ subgenre.strip!
+ tags = tags + Array(subgenre.downcase)
+ end
+ Song.new(song, artist, genre, subgenre, tags)
+ end
+end
- Константата
ArtistTags
вCollection
няма голям смисъл. Това е част от примерния ни тест, наистина, ама нали. - Не спазваш идентацията на две места - в
ArtistTags
и вconstruct_song
. Протокола изисква да ти взема точка за това. -
subgenre = subgenre.strip
е по-добре отsubgenre.strip!
. Ако има друга референция къмsubgenre
(това лесно се случва), вероятността за бъг ще е по-малка. - Слагат се скоби от вътрешната страна на блоковете -
each { |x| foo(x) }
, а неeach {|x| foo(x)}
. - Няма нужда да ползваш
%r{\.\s*}
./\.\s*/
е по-подходящо (и по-кратко).%r
се ползва само ако ти трябва\
в регулярния израз. - Имаш разхвърляни
p
-та из кода, което не е ОК. - Няма нужда да правиш
expected = Array(expected) if expected.kind_of? String
. Може простоArray(expected)
. Това му е идеята. -
verify_value
може да се сведе до точно един ред. Условието наif
-а. Простоdef verify_value(value, expected) expected == "" or value == expected end
. Ще стане доста по-добре. - Във
verify_tags
си искала да ползвашall?
илиany?
.
Дам.. усетих се за тези неща. Само идентацията ми е странно защо е счупена. Навсякъде съм идентирала с 2 спейса.
Между другото, скептика не може ли да проверява за нарушения на конвенцията? Би било полезно... случвало ми се е да се ровя из нета за това... случвало ми се е и да се подвеждам от примери в нета :(
Би било полезно, но е доста времеемко да се напише.
Впрочем, ето популярен style guide.