Решение на Втора задача от Мартин Асенов

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

Към профила на Мартин Асенов

Резултати

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

Код

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

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

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

Finished in 0.55192 seconds
12 examples, 0 failures

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

Мартин обнови решението на 30.10.2011 00:15 (преди над 12 години)

+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? на места, на които може.