Решение на Втора задача от Илиян Бобев

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

Към профила на Илиян Бобев

Резултати

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

Код

class Song
attr_accessor :name, :artist, :album
def initialize(args)
@name = args[0]
@artist = args[1]
@album = args[2]
end
end
class Criteria
attr_accessor :value
def initialize(&block)
@value = block
end
def self.name(arg)
Criteria.new { |song| song.name == arg }
end
def self.artist(arg)
Criteria.new { |song| song.artist == arg }
end
def self.album(arg)
Criteria.new { |song| song.album == arg }
end
def &(other)
Criteria.new { |song| @value.call(song) and other.value.call(song) }
end
def |(other)
Criteria.new { |song| @value.call(song) or other.value.call(song) }
end
def !
Criteria.new { |song| not @value.call(song) }
end
end
class Collection
include Enumerable
attr_accessor :storage
def initialize(data)
@storage = data
end
def each
@storage.each { |item| yield item }
end
def self.parse(data)
song_list = []
data.split("\n\n").each { |song| song_list << Song.new(song.split("\n")) }
Collection.new song_list
end
def filter(conditions)
Collection.new @storage.select { |song| conditions.value.call(song) }
end
def adjoin(filtered)
Collection.new (@storage + filtered.storage).uniq
end
def artists
@storage.collect { |song| song.artist }.uniq
end
def albums
@storage.collect { |song| song.album }.uniq
end
def names
@storage.collect { |song| song.name }.uniq
end
end

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

...........

Finished in 0.00997 seconds
11 examples, 0 failures

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

Илиян обнови решението на 30.10.2012 02:01 (преди над 11 години)

+class Song
+ attr_accessor :name, :artist, :album
+
+ def initialize(args)
+ @name = args[0]
+ @artist = args[1]
+ @album = args[2]
+ end
+end
+
+class Criteria
+ attr_accessor :value
+
+ def initialize(&block)
+ @value = block
+ end
+
+ def self.name(arg)
+ Criteria.new { |song| song.name == arg }
+ end
+
+ def self.artist(arg)
+ Criteria.new { |song| song.artist == arg }
+ end
+
+ def self.album(arg)
+ Criteria.new { |song| song.album == arg }
+ end
+
+ def &(other)
+ Criteria.new { |song| @value.call(song) and other.value.call(song) }
+ end
+
+ def |(other)
+ Criteria.new { |song| @value.call(song) or other.value.call(song) }
+ end
+
+ def !
+ Criteria.new { |song| not @value.call(song) }
+ end
+end
+
+class Collection
+ include Enumerable
+ attr_accessor :storage
+
+ def initialize(data)
+ @storage = data
+ end
+
+ def each
+ @storage.each { |item| yield item }
+ end
+
+ def self.parse(data)
+ songList = []
+ data.split("\n\n").each { |song| songList << Song.new(song.split("\n")) }
+ Collection.new songList
+ end
+
+ def filter(conditions)
+ Collection.new @storage.select { |song| conditions.value.call(song) }
+ end
+
+ def adjoin(filtered)
+ Collection.new (@storage + filtered.storage).uniq
+ end
+
+ def artists
+ @storage.collect { |song| song.artist }.uniq
+ end
+
+ def albums
+ @storage.collect { |song| song.album }.uniq
+ end
+
+ def names
+ @storage.collect { |song| song.name }.uniq
+ end
+end

Илиян обнови решението на 30.10.2012 22:46 (преди над 11 години)

class Song
attr_accessor :name, :artist, :album
def initialize(args)
@name = args[0]
@artist = args[1]
@album = args[2]
end
end
class Criteria
attr_accessor :value
def initialize(&block)
@value = block
end
def self.name(arg)
Criteria.new { |song| song.name == arg }
end
def self.artist(arg)
Criteria.new { |song| song.artist == arg }
end
def self.album(arg)
Criteria.new { |song| song.album == arg }
end
def &(other)
Criteria.new { |song| @value.call(song) and other.value.call(song) }
end
def |(other)
Criteria.new { |song| @value.call(song) or other.value.call(song) }
end
def !
Criteria.new { |song| not @value.call(song) }
end
end
class Collection
include Enumerable
attr_accessor :storage
def initialize(data)
@storage = data
end
def each
@storage.each { |item| yield item }
end
def self.parse(data)
- songList = []
- data.split("\n\n").each { |song| songList << Song.new(song.split("\n")) }
- Collection.new songList
+ song_list = []
+ data.split("\n\n").each { |song| song_list << Song.new(song.split("\n")) }
+ Collection.new song_list
end
def filter(conditions)
Collection.new @storage.select { |song| conditions.value.call(song) }
end
def adjoin(filtered)
Collection.new (@storage + filtered.storage).uniq
end
def artists
@storage.collect { |song| song.artist }.uniq
end
def albums
@storage.collect { |song| song.album }.uniq
end
def names
@storage.collect { |song| song.name }.uniq
end
end
  • args не ми изглежда разбираемо. Например attributes е по-добре. Аз бих променил цялото извикване да изглежда така: Song.new(*song.split("\n")) и конструктура - def initialize(name, artist, album).
  • Вместо да си пишеш сам Song класа можеш да напишеш Song = Struct.new(:name, :artist, :album).
  • При value отново не се разбира какво си имал в предвид, защото явно има метод call, но никак не е ясно какъв е този обект от името му. Ако беше condition, вероятно щеше да е много по-лесно четимо.
  • Името на атрибута storage също не е подходящо. Например songs би било по-подходящо.
  • Вместо да мутираш в следния фрагмент data.split("\\n\\n").each { |song| song_list << Song.new(song.split("\\n")) }, можеш да map-ваш. Фрагмента би изглежал така: data.split("\\n\\n").map { |song| Song.new(song.split("\\n")) }. По този начин кода е много по-четим, защото при map е ясно какво се прави - на всеки елемент от едно множество се съпоставя точно един друг елемент (не задължително различен). При each това не е толкова ясно. Освен това е прието за този вид операции да се ползва map и ако се използва нещо друго, то човек би се опитал да разбере защо. Има ли спициални причини за това? Има ли елемент, който се пропуска? Като цяло е добра практика да не мутираш в each.
  • Може да добавиш метод на Criteria, който да ти казва дали дадена песен изпълнява критерия (matches? например). По този начин няма да трябва да разкрива част от вътрешното си представяне. Също така, ако послушаш съвета ми и преименуваш value на condition сега ще трябва да пипаш на много местар Ако имаше такъв метод, щеше да трябва да промениш само него.
  • attr_accessor не ти трябва, за да достъпваш атрибути вътре в класа. Можеш да махнеш attr_accessor :storage.

само махането на attr_accessor :storage (което вече е songs) не се получи - изгърмя на adjoin където викам атрибута през инстанция

Song = Struct.new(:name, :artist, :album)

class Criteria
  attr_accessor :condition

  def initialize(&block)
    @condition = block
  end

  def self.name(predicament)
    Criteria.new { |song| song.name == predicament }
  end

  def self.artist(predicament)
    Criteria.new { |song| song.artist == predicament }
  end

  def self.album(predicament)
    Criteria.new { |song| song.album == predicament }
  end

  def &(other)
    Criteria.new { |song| @condition.call(song) and other.condition.call(song) }
  end

  def |(other)
    Criteria.new { |song| @condition.call(song) or other.condition.call(song) }
  end

  def !
    Criteria.new { |song| not @condition.call(song) }
  end

  def matches?(song)
    condition.call(song)
  end
end

class Collection
  include Enumerable
  attr_accessor :songs

  def initialize(data)
    @songs = data
  end

  def each
    @songs.each { |item| yield item }
  end

  def self.parse(data)
    Collection.new data.split("\n\n").map { |song| Song.new(*song.split("\n")) }
  end

  def filter(conditions)
    Collection.new @songs.select { |song| conditions.matches?(song) }
  end

  def adjoin(filtered)
    Collection.new (@songs | filtered.songs)
  end

  def artists
    @songs.collect { |song| song.artist }.uniq
  end

  def albums
    @songs.collect { |song| song.album }.uniq
  end

  def names
    @songs.collect { |song| song.name }.uniq
  end
end