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

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

Решение на Втора задача от Ивайло Петров

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

Към профила на Ивайло Петров

Резултати

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

Код

class Song
attr_accessor :name, :artist, :genre, :subgenre, :tags
def initialize(name, artist, genre, subgenre, tags)
@name, @artist, @genre, @subgenre = name, artist, genre, subgenre
@tags = tags
end
def matches_criteria?(criteria)
criteria.all? do |method_name, value|
method = method_name.to_s + '_matches?'
send method, value
end
end
private
def tags_matches?(tags)
# [*tags] is used instead of [tags], because tags might be 'something' or
# ['1', '2'], however we want in both cases to process array
tags = [*tags]
# first we check that all the tags that should be there, are there,
# then we check that all the tags, that shouldn't be there, are not there
positive_tags = tags.select { |tag| !tag.end_with? '!'}
negative_tags = tags.select { |tag| tag.end_with? '!'}
positive_tags.all? { |tag| @tags.include? tag } and
negative_tags.none? { |tag| @tags.include? tag[0..-2] }
end
def name_matches?(song_name)
@name == song_name
end
def artist_matches?(song_artist)
@artist == song_artist
end
def filter_matches?(filter_block)
filter_block.(self)
end
end
class Collection
SUBPROPERTY_DELIMITER = /,\s*/
def initialize(songs_as_string, artist_tags)
# Make artist_tags to have default value []
artist_tags = Hash.new([]).merge(artist_tags)
# parse songs from songs_as_string
@songs = songs_as_string.split("\n").map do |song_as_string|
parse_song(song_as_string, artist_tags)
end
end
def find(criteria)
@songs.select do |song|
song.matches_criteria? criteria
end
end
private
def parse_song(song_as_string, artist_tags)
song_as_array = generate_song_array(song_as_string)
name, artist = song_as_array[0..1]
genres = parse_genres(song_as_array[2])
tags = generate_tags(song_as_array[3], genres, artist, artist_tags)
Song.new(name, artist, genres[0], genres[1], tags)
end
def generate_song_array(song_as_string)
song_as_array = song_as_string.split('.')
# remove any whitespace, that we don't need
song_as_array.map! { |property| property.lstrip }
# if we don't have tags, song_as_array[3] will be nil
# which would be a problem
song_as_array[3] ||= ''
song_as_array
end
def parse_genres(genres_as_string)
genres = genres_as_string.split(SUBPROPERTY_DELIMITER)
end
def generate_tags(tags_as_string, genres, artist, artist_tags)
tags = tags_as_string.split(SUBPROPERTY_DELIMITER)
tags + genres_to_tags(genres) + artist_tags[artist]
end
def genres_to_tags(genres)
genres.map { |genre| genre.downcase }
end
end

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

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

Finished in 0.5421 seconds
12 examples, 0 failures

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

Ивайло обнови решението на 25.10.2011 20:33 (преди около 13 години)

+class Song
+ attr_accessor :name, :artist, :genre, :subgenre, :tags
+
+ def initialize(name, artist, genre, subgenre, tags)
+ @name, @artist, @genre, @subgenre = name, artist, genre, subgenre
+ @tags = tags
+ end
+
+ def matches_criteria?(criteria)
+ criteria.all? do |method_name, value|
+ method = method_name.to_s + '_matches?'
+ send method, value
+ end
+ end
+
+ private
+
+ def tags_matches?(tags)
+ # [*tags] is used instead of [tags], because tags might be 'something' or
+ # ['1', '2'], however we want in both cases to process array
+ tags = [*tags]
+
+ # first we check that all the tags that should be there, are there,
+ # then we check that all the tags, that shouldn't be there, are not there
+ positive_tags = tags.select { |tag| !tag.end_with? '!'}
+ negative_tags = tags.select { |tag| tag.end_with? '!'}
+ positive_tags.all? { |tag| @tags.include? tag } and
+ negative_tags.none? { |tag| @tags.include? tag[0..-2] }
+ end
+
+ def name_matches?(song_name)
+ @name == song_name
+ end
+
+ def artist_matches?(song_artist)
+ @artist == song_artist
+ end
+
+ def filter_matches?(filter_block)
+ filter_block.(self)
+ end
+end
+
+class Collection
+ SUBPROPERTY_DELIMITER = /,\s*/
+
+ def initialize(songs_as_string, artist_tags)
+ # Make artist_tags to have default value []
+ artist_tags = Hash.new([]).merge(artist_tags)
+
+ # parse songs from songs_as_string
+ @songs = songs_as_string.split("\n").map do |song_as_string|
+ parse_song(song_as_string, artist_tags)
+ end
+ end
+
+ def find(criteria)
+ @songs.select do |song|
+ song.matches_criteria? criteria
+ end
+ end
+
+ private
+
+ def parse_song(song_as_string, artist_tags)
+ song_as_array = generate_song_array(song_as_string)
+ name, artist = song_as_array[0..1]
+ genres = parse_genres(song_as_array[2])
+ tags = generate_tags(song_as_array[3], genres, artist, artist_tags)
+ Song.new(name, artist, genres[0], genres[1], tags)
+ end
+
+ def generate_song_array(song_as_string)
+ song_as_array = song_as_string.split('.')
+
+ # remove any whitespace, that we don't need
+ song_as_array.map! { |property| property.lstrip }
+
+ # if we don't have tags, song_as_array[3] will be nil
+ # which would be a problem
+ song_as_array[3] ||= ''
+
+ song_as_array
+ end
+
+ def parse_genres(genres_as_string)
+ genres = genres_as_string.split(SUBPROPERTY_DELIMITER)
+ end
+
+ def generate_tags(tags_as_string, genres, artist, artist_tags)
+ tags = tags_as_string.split(SUBPROPERTY_DELIMITER)
+ tags + genres_to_tags(genres) + artist_tags[artist]
+ end
+
+ def genres_to_tags(genres)
+ genres.map { |genre| genre.downcase }
+ end
+end

Ако погледнеш внимателно, ще видиш че Collection има няколко метода, които не ползват нищо от инстанцията - tags, name и прочее. Още повече, всички вземат един допълнителен аргумент (song) и боравят с него.

def name(song, song_name)
  song.name == song_name
end

def artist(song, song_artist) 
  song.artist == song_artist
end

def filter(song, filter_block)
  filter_block.(song)
end

Въобще, опитват се да ти кажат, че искат да ги преместиш от Collection в Song. Всеки път, когато имаш метод, който не борави по никакъв начин със self, това е ясен знак, че метода трябва да бъде някъде другаде.

Отделно, в тази задача имаш тамън две неща да правиш - (1) парсене на вход и (2) проверка дали песен отговаря на критерии. Тези неща трябва да стоят отделно. Първото е много подходящо за Collection, докато второто - за Song.

Не, не, не, не, и не.

Нещата с @@ в Ruby са класови променливи. Семантиката им е малко странна, което е причината да не сме говорили за тях. Пишат се със snake case. Това което търсиш е константа:

SUBPROPERTY_DELIMITER = /,\s*/
  • [*tags] е хитро, браво че си се сетил. Аз не се.
  • Браво, че си открил send. Получило се е хубаво.
  • tag[0..2] може да се запише като tag.chomp('!') в този случай.
  • По принцип е по-добре да ползваш: filter_block.call, отколкото filter_block.(). Можеше просто да го кръстиш filter.
  • Този map! не е особено адекватен. По-добре си създай нов списък.
  • Аз не бих изкарал SUBPROPERTY_DELIMITER. Бих го inline-нал и на двете места.
  • Може да напишеш send "#{method_name}_matches?", value. По-удачно е.
  • genres.map(&:downcase) е по-добре от genres.map { |genre| genre.downcase }.

Иначе, решението ти ми харесва. Личи си, че си понаучил доста Ruby и одобрявам. Имаш 2 бонус точки за това.