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

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

Решение на Втора задача от Борис Русев

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

Към профила на Борис Русев

Резултати

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

Код

class Song
attr_accessor :name
attr_accessor :artist
attr_accessor :genre
attr_accessor :subgenre
attr_accessor :tags
def pass?(including_criteria, excluding_tags)
may_include? including_criteria and not should_exclude? excluding_tags
end
def may_include?(including_criteria)
passes = 0
including_criteria.each do |key, value|
case value
when String then passes += 1 if send(key) == value
when Array then passes += 1 if (value - send(key)).empty?
end
end
including_criteria.length == passes
end
def should_exclude?(excluding_tags)
excluding_tags.any? { |tag| send(:tags).include? tag }
end
end
class Collection
def initialize(songs_as_string, artist_tags)
@songs_as_string, @artist_tags = songs_as_string, artist_tags
@songs = @songs_as_string.lines.map { |item| parse_song(item.chomp) }
end
def find(criteria)
if criteria.include? :filter
return @songs.select { |song| criteria[:filter].call(song) }
end
criteria[:tags] = Array(criteria[:tags])
excluding_tags = flatten_array(excluding_tags(criteria))
including_criteria = criteria
unless criteria[:tags].nil?
including_tags = flatten_array(criteria[:tags]) - excluding_tags
including_criteria[:tags] = including_tags
end
@songs.select { |song| song.pass?(including_criteria, excluding_tags) }
end
private
def parse_song(song_string)
song = Song.new
song_data = song_string.split(".").map(&:lstrip)
song.name = song_data[0]
song.artist = song_data[1]
genres = song_data[2].split(",").map(&:lstrip)
song.genre = genres[0]
song.subgenre = genres[1]
song.tags = generate_tags(song)
unless song_data[3].nil?
song.tags = song.tags + song_data[3].split(",").map(&:lstrip)
end
song
end
def generate_tags(song)
tags = []
tags << song.genre.downcase
tags << song.subgenre.downcase unless song.subgenre.nil?
if @artist_tags.include? song.artist
tags << @artist_tags[song.artist].map(&:downcase)
end
tags.flatten
end
def excluding_tags(criteria)
return [] if criteria[:tags].nil?
excluding_tags = []
criteria[:tags].each do |tag|
if tag.end_with? "!"
excluding_tags << tag
end
end
excluding_tags
end
def flatten_string(string)
if string.end_with? "!"
string.chop
else
string
end
end
def flatten_array(array)
array.map { |item| flatten_string(item) }
end
end

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

..........F.

Failures:

  1) Collection allows all criteria
     Failure/Error: ).map(&:artist).should eq ['Thelonious Monk']
       
       expected: ["Thelonious Monk"]
            got: ["John Coltrane", "John Coltrane", "John Coltrane", "John Coltrane", "John Coltrane", "John Coltrane", "Miles Davis", "Miles Davis", "Miles Davis", "Miles Davis", "Bill Evans", "Bill Evans", "Thelonious Monk", "Thelonious Monk"]
       
       (compared using ==)
       
       Diff:
       @@ -1,2 +1,15 @@
       -["Thelonious Monk"]
       +["John Coltrane",
       + "John Coltrane",
       + "John Coltrane",
       + "John Coltrane",
       + "John Coltrane",
       + "John Coltrane",
       + "Miles Davis",
       + "Miles Davis",
       + "Miles Davis",
       + "Miles Davis",
       + "Bill Evans",
       + "Bill Evans",
       + "Thelonious Monk",
       + "Thelonious Monk"]
     # /tmp/d20111115-13548-19o3oat/spec.rb:82:in `block (2 levels) in <top (required)>'
     # ./lib/homework/run_with_timeout.rb:5:in `block (3 levels) in <top (required)>'
     # ./lib/homework/run_with_timeout.rb:5:in `block (2 levels) in <top (required)>'

Finished in 0.55005 seconds
12 examples, 1 failure

Failed examples:

rspec /tmp/d20111115-13548-19o3oat/spec.rb:76 # Collection allows all criteria

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

Борис обнови решението на 30.10.2011 11:27 (преди около 13 години)

+class Song
+ attr_accessor :name
+ attr_accessor :artist
+ attr_accessor :genre
+ attr_accessor :subgenre
+ attr_accessor :tags
+
+ def pass?(including_criteria, excluding_tags)
+ may_include? including_criteria and not should_exclude? excluding_tags
+ end
+
+ def may_include?(including_criteria)
+ passes = 0
+
+ including_criteria.each do |key, value|
+ case value
+ when String then passes += 1 if send(key) == value
+ when Array then passes += 1 if (value - send(key)).empty?
+ end
+ end
+
+ including_criteria.length == passes
+ end
+
+ def should_exclude?(excluding_tags)
+ excluding_tags.any? { |tag| send(:tags).include? tag }
+ end
+end
+
+class Collection
+ def initialize(songs_as_string, artist_tags)
+ @songs_as_string, @artist_tags = songs_as_string, artist_tags
+ @songs = @songs_as_string.lines.map { |item| parse_song(item.chomp) }
+ end
+
+ def find(criteria)
+ if criteria.include? :filter
+ return @songs.select { |song| criteria[:filter].call(song) }
+ end
+
+ criteria[:tags] = Array(criteria[:tags])
+
+ excluding_tags = flatten_array(excluding_tags(criteria))
+ including_criteria = criteria
+ unless criteria[:tags].nil?
+ including_tags = flatten_array(criteria[:tags]) - excluding_tags
+ including_criteria[:tags] = including_tags
+ end
+
+ @songs.select { |song| song.pass?(including_criteria, excluding_tags) }
+ end
+
+ private
+
+ def parse_song(song_string)
+ song = Song.new
+ song_data = song_string.split(".").map(&:lstrip)
+
+ song.name = song_data[0]
+ song.artist = song_data[1]
+
+ genres = song_data[2].split(",").map(&:lstrip)
+
+ song.genre = genres[0]
+ song.subgenre = genres[1]
+
+ song.tags = generate_tags(song)
+
+ unless song_data[3].nil?
+ song.tags = song.tags + song_data[3].split(",").map(&:lstrip)
+ end
+
+ song
+ end
+
+ def generate_tags(song)
+ tags = []
+
+ tags << song.genre.downcase
+ tags << song.subgenre.downcase unless song.subgenre.nil?
+ if @artist_tags.include? song.artist
+ tags << @artist_tags[song.artist].map(&:downcase)
+ end
+
+ tags.flatten
+ end
+
+ def excluding_tags(criteria)
+ return [] if criteria[:tags].nil?
+
+ excluding_tags = []
+
+ criteria[:tags].each do |tag|
+ if tag.end_with? "!"
+ excluding_tags << tag
+ end
+ end
+
+ excluding_tags
+ end
+
+ def flatten_string(string)
+ if string.end_with? "!"
+ string.chop
+ else
+ string
+ end
+ end
+
+ def flatten_array(array)
+ array.map { |item| flatten_string(item) }
+ end
+end
  • Ruby се идентира с два интервала, а не с табове.
  • String#end_with? е по-добре, от tag.rindex('!') == tag.size -1
  • Дълги, едноредови if-ове с else са гадни. Запиши #flatten_string така: if string.rindex("!") == string.size - 1 string.chop else string end
  • Около скобите на блоковете в Ruby се слагат интервали: array.map { |item| flatten_string(item) }
  • should_exclude? може да се имплементира с excluding_tags.any? do |tag|. Така ще се чете по-лесно и ще бъде по-кратко.
  • Не си погледнал какво правят Array('foo') и Array(['foo', 'bar']). Ако погледнеш, може да опростиш част от кода си.
  • Част от методите в Collection би трябвало да са private.
  • song_string.lines е далеч по-добре от song_string.split("\n").

Стефан, благодаря ти за предварителният коментар по кода, въпреки че направо се стресирах колко много неща не са ти харесали. Каквото мога ще поправя, но все пак да отбележа някои неща по ред на забележките ти.

1) За идентацията не го разбирам? Ползвам gedit и ползвам табове да, но съм го настроил табулацията да ми е 2. Какво не правя както трябва?

2) Това за едноредовият if...първоначалната версия на кода ми беше точно такава както ти си го написал, но мислех че в Руби се предпочитат едноредовите, ще го имам предвид :)

3) Разстояния около скобите на блоковете също съм сложил, погледнах си кода, не знам какво се е объркало тука...

4) Прав си не съм погледнал подсказките...нямам оправдание за там

5) song_string.split("\n") пробвах да го заменя със song_string.lines защото е по-интуитивно и би трябвало да е аналогично...но не се получи, тестовете ми фейлнаха когато го направих, а не че не съм се сетил за него, беше в подсказката

Отново благодаря :)

1) Ок оправих табулацията, било е от настройките на gedit 2) lines се оказа че не работело щото накрая поставяло \n което не се сетих да го погледна в документацията 3) За подсказката още не чаткам, ама нищо като излязат решенията на колегите ще разбера какво точно си имал предвид :)

Нека да си дам моите две стотинки за lines. Не че не се бях сетил за него, но според мене е по-добре да се ползва split. Защото първо split е по-разпознаваемо, мисля че се среща по-често в другите платформи. От друга страна lines добавя \n което си е излишно и после трябва да го коригирам.

Така и така трябва да коригираш новя ред накрая със strip, понеже нищо не ти гарантира, че няма trailing и leading whitespace. Можеш да напишеш string.split(/\s*\n\s*/), но така читателя трябва да парси регулярен израз string.lines.map(&:strip) е по-разпознаваемо.

"среща се по-често в другите платформи" не е критерий за четим код, както опитах да обясня в понеделник.

Ето още няколко коментара:

  • unless criteria[:tags].nil? е доста по-криптично от по-простото if criteria[:tags].
  • Брояча в may_include? е странен. Там трябваше да минеш просто с един #all?.
  • Инициализираш празен Song и после му викаш setter-и. Щеше да е по-добре просто да подадеш петте параметъра в конструктора на Song. Може би дори като хеш.