Програмиране с Ruby

Курс във Факултета по Математика и Информатика към СУ

Решение на Втора задача от Георги Христозов

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

Към профила на Георги Христозов

Резултати

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

Код

class Song
attr_accessor :name, :artist, :genre, :subgenre, :tags
def parse string_to_parse, artist_tags
@name = string_to_parse[0]
@artist = string_to_parse[1]
genrestring = string_to_parse[2].split(',').map(&:strip)
@genre = genrestring[0]
@tags = [@genre.downcase]
if genrestring[1] != nil
@subgenre = genrestring[1]
@tags << @subgenre.downcase
end
if string_to_parse[3] != nil
@tags.concat(string_to_parse[3].split(',').map(&:strip))
end
@tags.concat(artist_tags[@artist])
end
def matches? criteria, value
case criteria
when :artist
matches_artist? value
when :name
matches_name? value
when :tags
matches_tag? value
when :filter
matches_filter? value
end
end
def matches_tag? tag
if tag.kind_of? String
if tag[-1] == "!"
@tags.index(tag[0...-1]) == nil
else
@tags.index(tag) != nil
end
elsif tag.kind_of? Array
result = true
tag.each do |tag_element|
result &= matches_tag? tag_element
end
result
end
end
def matches_name? name
@name == name
end
def matches_artist? artist
@artist == artist
end
def matches_filter? filterfunc
filterfunc.call(self)
end
end
class Collection
def initialize songs_as_string, artist_tags
artist_tags.default = []
@songs = songs_as_string.split("\n")\
.map{|line| line.split('.').map &:strip}\
.map{|string_for_parsing|
song = Song.new
song.parse(string_for_parsing,artist_tags)
song
}
end
def find hash
hash.inject(@songs) {|res, criteria|
res &= @songs.select{|song| song.matches? criteria.first, criteria.last}
res
}
end
end

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

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

Finished in 0.54839 seconds
12 examples, 0 failures

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

Георги обнови решението на 25.10.2011 13:27 (преди около 14 години)

+class Song
+ attr_accessor :name, :artist, :genre, :subgenre, :tags
+
+ def parse string_to_parse, artist_tags
+ @name = string_to_parse[0]
+ @artist = string_to_parse[1]
+ genrestring = string_to_parse[2].split(',').map(&:strip)
+ @genre = genrestring[0]
+ @tags = [@genre.downcase]
+ if genrestring[1] != nil
+ @subgenre = genrestring[1]
+ @tags << @subgenre.downcase
+ end
+ if string_to_parse[3] != nil
+ @tags.concat(string_to_parse[3].split(',').map(&:strip))
+ end
+ @tags.concat(artist_tags[@artist])
+ end
+
+ def matches? criteria, value
+ case criteria
+ when :artist
+ matches_artist? value
+ when :name
+ matches_name? value
+ when :tags
+ matches_tag? value
+ when :filter
+ matches_filter? value
+ end
+ end
+
+ def matches_tag? tag
+ if tag.kind_of? String
+ if tag[-1] == "!"
+ @tags.index(tag[0...-1]) == nil
+ else
+ @tags.index(tag) != nil
+ end
+ elsif tag.kind_of? Array
+ result = true
+ tag.each do |tag_element|
+ result &= matches_tag? tag_element
+ end
+ result
+ end
+ end
+
+ def matches_name? name
+ @name == name
+ end
+
+ def matches_artist? artist
+ @artist == artist
+ end
+
+ def matches_filter? filterfunc
+ filterfunc.call(self)
+ end
+end
+
+class Collection
+ def initialize songs_as_string, artist_tags
+ artist_tags.default = []
+ @songs = songs_as_string.split("\n")\
+ .map{|line| line.split('.').map &:strip}\
+ .map{|string_for_parsing|
+ song = Song.new
+ song.parse(string_for_parsing,artist_tags)
+ song
+ }
+ end
+
+ def find hash
+ hash.inject(@songs) {|res, criteria|
+ res &= @songs.select{|song| song.matches? criteria.first, criteria.last}
+ res
+ }
+ end
+end

У, написах един коментар два пъти. Може би трябва триене.

Както и да е, друго което можеш да направиш с тези ламбди е да ги плъзнеш като константа някъде.

Или далеч по-умното, можеш да ги направиш методи на Song. Една песен може сама да си прецени дали отговаря на някакви критерии или не. Можеш да кръстиш метода #matches? и после да филтрираш масива с песни от Collection с нея.

Парсенето на файла може да остане в Collection.

  • genrestring се пише като genre_string
  • tag.end_with? '!', вместо tag[-1] == '!'. Последното няма да работи в 1.8.
  • Това в matches_tag? се пише с tag.all? { |tag_element| matches_tag? tag_element }.
  • Цялостното ти подравняване в Collection#initialize е доста страно.
    • Няма нужда от \ на края на реда
    • whitespace-а се слага така .map { |line line.split }
  • Когато блока е на повече от един ред, се ползва do/end нотацията. Грешиш го на няколко места.
  • Това да имаш instance метод Song#parse е странно. Трябва да правиш song = Song.new; song.parse. Така можеш да инстанцираш невалиден Song, което не е хубаво. По-добре щеше:
    1. Да го прави конструктора. Song.new(string_for_parsing).
    2. Да бъде class метод в Song. Song.parse(string_for_parsing).
    3. Да го прави Collection.
  • Начина, по който ползваш #inject в Collection#find е ужасен. Там е за Enumerable#select, в който да има #all?.

Щях да ти взема точка, но щях и да ти дам точка за кратко решение.

Питай, ако имаш въпроси.

Без наклонената черта в края на реда, vim не ми изместваше вдясно продължението на израза на новия ред. Логиката ми беше, че щом vim се държи така, явно липсата на \ не е добра практика. Тъпо.

А за parse-ването - то първоначално беше в Collection-а, но след първия ти коментар го преместих в Song. Лошото е, че след още малко промени направих метода твърде дебел и не можех да го върна в Collection. Щях да наруша ограниченията, защото с конструирането и връщането на Song ставаше прекалено дълъг. А вариантът за разделяне на няколко метода не ми допадаше. Сега гледам, че има много изродски решения на този проблем.

А колкото до inject-а - с нещо подобно се озовах на "слайдовете на срама" след първото домашно. Въпреки това, в това Collection#find не ми изглеждаше твърде обфускирано. Когато го писах все още имах разбирах критериите като "дизюнктивни" и след като поправих това нещата явно доста са се замазали. Дори и сега, като погледнах кода, ми се наложи да се замисля за момент, за да разбера какво се случва.

За останалата критика - ще гледам да си науча урока и да не се повтаря. :)