• 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