Ruby中的多集实现

Question

我正在尝试实现维基百科定义的MultiSet，并需要有关实现的反馈。

class MultiSet
  include Enumerable

  attr_reader :members

  def initialize enum={}
    @members = {}
    raise_error unless enum.class.include? Enumerable
    enum.each do |item|
      if @members.include?(item) 
        @members[item] += 1
      else
        @members[item] = 1
      end      
    end  
  end

  def each(&blk)
    @members.each(&blk)
  end

  def == other
    members.to_h == other.to_h 
  end

  def eql? other
    self == other
  end

  def to_h
    members.dup
  end  

  def to_set
    Set.new @members.keys
  end  

  def remove item
    if @members.include?(item) 
      @members[item] = @members[item] - 1
      if @members[item] < 1
        @members.delete(item)
      end
    end      
    self
  end 

  def add item
    if @members.include?(item) 
      @members[item] += 1      
    else
      @members[item] = 1
    end        
    self
  end 

  def empty!   
    @members.clear    
  end 

  def multiplicity item 
    @members[item] == 0 ? nil : @members[item]
  end

  def include? item
    @members.include? item 
  end

  def cardinality
    return 0 if @members.empty?
    @members.values.reduce(:+)
  end 

  def | other
    other.each do |k,v|
      if members.include? k
        members[k] = multiplicity(k) + other.multiplicity(k)
      else
        add k
      end     
    end
    self
  end   

  def & other
    members.each do |k,v|
      if other.include? k
        members[k] = [multiplicity(k), other.multiplicity(k)].min
      else
        remove k
      end     
    end 
    self
  end    

end

Answer 1

• 有时你跳过圆括号，有时你没有。请保持一致
• 有时直接访问@members，有时使用members读取器方法。在某些情况下，这样做是有意义的，但在这里它是随机的。再一次:保持一致。然而，在提供阅读器时要小心。现在，我可以说a_multiset.members[some_item] = -100或其他什么，事情会变得奇怪，因为我已经混乱的内部数据结构。#members访问器作为#to_h的别名可能更好
• raise_error是什么？我看不出它在任何地方都有定义。有趣的是，这意味着它确实会引发一个错误--这个错误告诉您没有定义raise_error。如果要引发错误，则引发适当的错误:引发ArgumentError，“枚举必须包括‘枚举’模块”
• enum.class.include?(Enumerable)是编写enum.kind_of?(Enumerable)的一种迂回方式。
• 初始化器复制#add方法中的逻辑；只需调用#add即可。
• #remove可以通过修复if members[item] < 1来稍微清理一下，这只是为了摆脱金字塔缩进。
• 您的#empty!方法可能应该称为#clear。这是Hash、Array和Set使用的传统名称(可以看出，因为这是您的方法在@members哈希上调用的)
• 非常肯定您的#multiplicity方法是完全向后的。如果一个项目的计数为零，则返回nil。我认为您想要的正好相反:如果项不存在，则返回零:包括？(项)？成员项目：0
• 在可能的时候使用你自己的方法。您已经有了一个#include?方法，所以不需要在所有地方都使用@members.include?。
• #cardinality可以只写为members.values.reduce(0，:+)，如果您向#reduce (0)提供了一个初始值(0)，则不需要额外的空值--检查您现在拥有的
• 很肯定你的联合方法也是不正确的。如果一个键不存在，您只需添加它-意味着它将有一个计数1。但实际上，它应该有相同的计数，它在另一组。和您也可以简化这个方法: def \(其他) other.each做_key，count ~ can 钥匙=多重性(键)+计数结束自结束
• 联合方法和交叉方法可能都应该返回一个新的MultiSet实例，而不是修改接收方。