couple.rb

inp = <<INP
A:c,b,a
B:a,b,d
C:a,c,b
D:d,a,c
a:A,C,D
b:D,A,B
c:B,A,C
d:D,C,A
INP

class Person
  attr_accessor :name, :hope, :ismale
  def initialize(data)
    @name, rank = data.split(':')
    @hope = rank.split(',')
    @ismale= @name =~ /[A-Z]/ ? true : false
  end
end

class Couple
  attr_accessor :point, :male, :female
  def initialize(m, f)
    @point = calc(m, f)
    @male = m
    @female = f
  end

  def calc(male, female)
    point =(male.hope.include?(female.name) ? male.hope.index(female.name) : 99) +(female.hope.include?(male.name) ? female.hope.index(male.name) : 99)
  end

  def str()
    "#{@male.name}-#{@female.name}:#{@point}"
  end
end

module Calc 
  def parse(input_data)
    input_data.split("\n").map(&Person.method(:new))
  end

  def generate_couple(people)
    people
    .combination(2)
    .select {|pair|
      pair[0].ismale != pair[1].ismale
    }
    .map {|pair|
      Couple.new(pair[0], pair[1])
    }
  end

  def pick(member)
    proc {|sorted|
      match = []
      sorted.each {|couple|
        if member.include?(couple.male.name) and member.include?(couple.female.name)
          match << couple
          member = (member - [couple.male.name, couple.female.name])
        end
      }
      match 
    }
  end

  module_function :parse, :generate_couple, :pick
end

people = Calc.parse(inp)
picker = Calc.pick(people.map {|p| p.name})
couples = Calc.generate_couple(people)
sorted = couples.sort {|a, b| a.point <=> b.point }

ans = picker.(sorted)
ans.each {|c|
  puts c.str
}

#Calc.parseとCalc.generate_coupleとpickerをArrowChoiceを使って合成できそう

@ismale= @name =~ /[A-Z]/ ? true : falseについて。

• @nameがあれば同じ結果が出せるので、インスタンス変数を使わずメソッド化する
• is_maleと単語はアンダーバーで区切る。というかRubyならmale?の方が自然
• @name =~ /[A-Z]/の戻り値自体がtrue/falseとして使えるので、わざわざtrue/falseを返さない

@point = calc(m, f)について。

• ここもやはり@male @femaleがあればいつでも算出できるのでメソッド化する
• initializeの中で複雑な処理をするとnewした瞬間にエラーが出たりして、ややこしい問題が起きがち。なので、あまりオススメしません。

  def calc(male, female)
    point =(male.hope.include?(female.name) ? male.hope.index(female.name) : 99) +(female.hope.include?(male.name) ? female.hope.index(male.name) : 99)
  end

について。

• point =は使われないローカル変数なので不要。
• male.hope.include?(female.name) ? male.hope.index(female.name) : 99 => male.hope.index(female.name) || 99
• 99のような値をフラグにしてしまうと99人以上の人数に対応できなくなるので、こういうフラグを使わない実装を考える。

  def generate_couple(people)
    people
    .combination(2)
    .select {|pair|
      pair[0].ismale != pair[1].ismale
    }
    .map {|pair|
      Couple.new(pair[0], pair[1])
    }
  end

について。

• .select {|one, other| one.ismale != other.ismale}みたいに書いた方がリーダブル。
• .map {|pair| Couple.new(*pair)}とも書けますよ。

  def pick(member)
    proc {|sorted|
      match = []
      sorted.each {|couple|
        if member.include?(couple.male.name) and member.include?(couple.female.name)
          match << couple
          member = (member - [couple.male.name, couple.female.name])
        end
      }
      match 
    }
  end

について。

• member = (member - [couple.male.name, couple.female.name])で、引数memberを変更するのはメソッドとしてサプライズ仕様ですね。引数に影響を与えない実装を考えてみてください。ほら、関数型言語なら変数の再代入を認めないでしょ？
• match = [] ... each ... matchのような処理はeach_with_objectで置き換えるのに打ってつけです。

• module_function :parse, :generate_couple, :pick => module_functionにするなら、Calcをクラスにして、各メソッドをクラスメソッドにしても同じだったのでは？

• people.map {|p| p.name} => people.map(&:name)と書けます。

• couples.sort {|a, b| a.point <=> b.point } => Coupleクラスに<=>メソッドを定義してcouples.sortと書く方がオブジェクト指向的です。

そして、最後にコードを実行してみて思ったのですが・・・出てきた答えが違うっ！！！
先に実行してからコードレビューすれば良かった(T T)