CodeIQのカラオケマシン問題を解いた

コミュニティ

ソニックガーデン社員によりCodeIQに出題されたカラオケマシン問題のベストコード発表会がUstreamで7/8に放送されます。

http://sonicgarden.doorkeeper.jp/events/12901

 

CodeIQの締め切りは既に済んでいたけれど、問題がブログで公開されていたので遅れながらも解答してみました。

 

※注意　以下の文章は、まだ上記のカラオケマシン問題を解くつもりで、解いていない人にはネタバレとなってしまいます。

 

以下に最初に作った解答コードを掲載します。 

# coding: utf-8

class KaraokeMachine
  TONES = ["C", "C#", "D", "D#", "E", "F", "F#", "G", "G#", "A", "A#", "B"]
	
  def initialize(melody)
    @melody = melody
  end

  def transpose(amount)
    melody = ""
    sharpFlag = false
    @melody.reverse.chars {|tone| 
      if tone == "#" then
        sharpFlag = true
        next
      end

      if sharpFlag then
        tone += "#"
	sharpFlag = false
      end

      index = TONES.index(tone)
      unless index.nil? then
        index += amount
	index %= TONES.length
        tone = TONES[index].reverse
      end
      melody += tone
    }
    return melody.reverse

  end
end

 

rubyの文字列操作の処理しやすさを活かしたとすると、今の私ではこれぐらい。

アルゴリズムを簡素化するために、データを後ろから処理するようにした。

 

上記のコードを見直してみたものを以下に掲載します。

他のブログに、解答例を公開している人がいたので、それも参考にしました。

 

# coding: utf-8

class KaraokeMachine
  TONES = %w(C C# D D# E F F# G G# A A# B)
	
  def initialize(melody)
    @melody = []
    melody.chars {|tone|
      if tone == "#" 
        @melody[-1] += "#"
        next
      end
      @melody.push(tone)
    }
  end

  def transpose(amount)
    melody = ""
    @melody.each {|tone| 
      if i = TONES.index(tone)
        i = (i+amount) % TONES.length
        tone = TONES[i]
      end
      melody += tone
    }
    return melody

  end
end

基本的なデータ構造とアルゴリズムは、参考にした他者のブログの解答コードともはや同じです。

これならば、データ後ろから処理する工夫も必要もない。また、分岐を減らすためelse句を極力使わない工夫は残しました。

これを超えるアイデアで根本的にコードを奇麗にするのは私には無理かな。

ベストコードの発表が楽しみになってきた。

 

 7/8追記

放送を視聴した上で、もう一度リファクタリングしたコードを以下に掲載します。

正規表現を使うアイデアで見直しました。

rubyの標準APIを駆使してハッシュで変換テーブルを使うアイデアが紹介されていましたが、ruby実務経験が無い私には難しかったかもしれません。性能改善の観点でも良さそうですね。

またrubyの文化ではreturnはあまり書かないそうです。

 

# coding: utf-8

class KaraokeMachine
  TONES = %w(C C# D D# E F F# G G# A A# B)
	
  def initialize(melody)
    @melody = melody
  end

  def transpose(amount)
    @melody.gsub(/[A-G]#?/){|tone| 
      i = (TONES.index(tone) + amount) % TONES.length
      TONES[i]
    }
  end
end

勉強になりました。

プロジェクトの地獄天国を分ける要因の一つはコードの簡潔さだと改めて思いました。