Мартин обнови решението на 30.10.2011 00:15 (преди около 13 години)
+class Song
+ attr_accessor :attributes
+ def initialize
+ @attributes = {}
+ end
+
+ def name
+ @attributes[:name][0]
+ end
+
+ def artist
+ @attributes[:artist][0]
+ end
+
+ def genre
+ @attributes[:genre][0]
+ end
+
+ def subgenre
+ if not @attributes[:subgenre]
+ @attributes[:subgenre]
+ else
+ if @attributes[:subgenre].length > 1
+ @attributes[:subgenre]
+ else
+ @attributes[:subgenre][0]
+ end
+ end
+ end
+
+ def tags
+ if not @attributes[:tags]
+ @attributes[:tags]
+ else
+ if @attributes[:tags].length > 1
+ @attributes[:tags]
+ else
+ @attributes[:tags][0]
+ end
+ end
+ end
+end
+
+class SongParser
+ def parse_and_attach_songs(catalogue, tags)
+ songs = []
+ catalogue.each_line { |line| songs << (parse_song line, tags)}
+ songs
+ end
+
+ private
+ def parse_song(song_as_string, tags)
+ song_data = song_as_string.split('.').map(&:strip)
+ song = Song.new
+ parse_song_name(song,song_data[0])
+ parse_artist_name(song,song_data[1])
+ parse_genres_and_subgenres(song,song_data[2])
+ parse_tags(song,song_data[3], tags)
+ song
+ end
+
+ private
+ def parse_song_name(song, song_name_as_string)
+ song.attributes[:name] = [song_name_as_string]
+ end
+
+ private
+ def parse_artist_name(song, artist_name_as_string)
+ song.attributes[:artist] = [artist_name_as_string]
+ end
+
+ private
+ def parse_genres_and_subgenres(song, genres_and_subgenres_as_string)
+ all_genres = genres_and_subgenres_as_string.split(',').map(&:strip)
+ song.attributes[:genre] = [all_genres[0]]
+ if all_genres.length > 1
+ song.attributes[:subgenre] = all_genres[1...all_genres.length]
+ end
+ end
+
+ private
+ def parse_tags(song, tags_as_string, tags_dict)
+ if not tags_as_string
+ song.attributes[:tags] = []
+ else
+ song.attributes[:tags] = tags_as_string.split(',').map(&:strip)
+ end
+ tags_dict.each do |key, value|
+ if key == song.artist
+ song.attributes[:tags].concat(value)
+ end
+ end
+ attach_genre_and_subgenres_as_tags(song)
+ end
+
+ private
+ def attach_genre_and_subgenres_as_tags(song)
+ song.attributes[:tags].concat(song.attributes[:genre].map(&:downcase))
+ if song.attributes[:subgenre]
+ song.attributes[:tags].concat(song.attributes[:subgenre].map(&:downcase))
+ end
+ end
+end
+
+class Collection
+ def initialize(catalogue, tags)
+ song_parser = SongParser.new
+ @songs = song_parser.parse_and_attach_songs(catalogue,tags)
+ end
+
+ def find(criteria={})
+ result = @songs.select{ |song| is_criteria_fulfilled?(song,criteria)}
+ end
+
+ def is_criteria_fulfilled?(song, criteria)
+ result = true
+ criteria.each do |key, value|
+ if value.kind_of? Array
+ result = result && is_complex_criteria_fulfilled?(song,key,value)
+ else
+ result = result && is_simple_criteria_fulfilled?(song,key,value)
+ end
+ end
+ result
+ end
+
+ private
+ def is_complex_criteria_fulfilled?(song, key, values)
+ result = true
+ values.each do |value|
+ result = result && is_simple_criteria_fulfilled?(song, key, value)
+ end
+ result
+ end
+
+ private
+ def is_simple_criteria_fulfilled?(song, key, value)
+ result = false
+ if key == :filter
+ result = value.call(song)
+ else
+ if value.include?("!")
+ result = (not song.attributes[key].include?(value.chop))
+ else
+ result = song.attributes[key].include?(value)
+ end
+ end
+ result
+ end
+end
-
private
няма нужда да бъде преди всеки метод. Достатъчно е веднъж, преди пъривияprivate
метод -
tags_dict
е странно име, понеже в Ruby не се наричатdict
-ове.tags_hash
е по-добре. -
Array#concat
е по-криптичен начин да напишешArray#+=
. -
SongParser
не е лоша идея, макар че:-
CollectionParser
е по-добро име -
CollectionParser.new(string).songs
е по-добър интерфейс.
-
-
result = []; string.each_line { result << ... }
е още известно катоstring.lines.map
-
if; else; if; end; end
може да се запише катоif; eslif; end
, което е по-добре -
#is_criteria_fulfilled?
може да ползваEnumerable#all?
. Далеч по-четимо е. - Във
#find
няма нужда отresult =
.
Имам и още, но засега толкова.
Ето още коментари.
-
Мутираш песента така.
song.attributes[:name] = 'Something'
. Далеч по-добре е да правишsong.name = 'Something
'. Допускам, че си го направил за да можеш да получиш хеш. Това можеше да стане ако ползвашattr_accessor
и в конструктора имаш:attributes.each do |name, value| send "#{name}=", value end
Това е популярен идиом.
-
SongParser
щеше да изглежда по-добре, ако първо конструираше хеша и после го подаваше наSong.new
. В момента конструираш една песен, която постепенно мутираш. Можеш да видиш малко Feature envy от страна наSongParser
. - Това че си направил
SongParser
ми хареса. Ограниченията ли те тласнаха натам. - Не ползваш
#all?
на места, на които може.