【プログラミング】よい命名規則とは。。

せかいさんのコードは綺麗じゃないね

というメールを師匠から貰って。せかいさんピーンチ！

　
くわしくはこちら。
【Ruby】リファクタリング。まじめに。 - せかいや

ということで、以前の師匠のコードを見返してみたよ。

師匠のやっていたリファクタリングは、、

tmp -> next_state
candidattes -> answers
seed_arr -> state
gene -> search

かな。

変数名をこしょこしょ変えているのが分かった。
わたしも分かりやすくするために、変数名を工夫しよう！

命名規則とは by ウィキペディア

Wikiを読む。
うーん。とくに発見はなかった。

でも何に気をつけたらいいのか掴めてきた。

変数名を省略しない
安易にtmpを使わない

調べてたところ、
コードコンプリートに変数命名が詳しく載っているようだ。
Amazon.co.jp： Code Complete第2版〈上〉―完全なプログラミングを目指して: スティーブマコネル, Steve McConnell, クイープ: 本

ちょうどこの本、持ってる。
早速読んでみよう。

現在のリファクタリング状況。
だいぶ良くなった。
　

def cases
  results = ["000000"] #人人人鬼鬼鬼が左岸
  6.times do |i|
    copy = results.dup
    copy.each do |x|
      new_state = x.dup
      new_state[i] = "1"
      results << new_state
    end
  end
  results
end

#食べられないケースだけ抽出
def sieve(cases)
  results = []
  cases.each do |caze|
    caze_to_int = caze.split("").map{|a|a.to_i}
    a = caze_to_int[0,3].inject(:+)
    b = caze_to_int[3,3].inject(:+)
    #全人間がどちらかの端に存在 もしくは 左岸&&右岸がOK
    results << caze if (a==3 || a==0) || ((3-a) >= (3-b) && a>=b)
  end
  results
end

def possible?(state, next_state)
  return false if state == next_state
  mover = 0
  if @history.length%2 == 0 #右から左へ
    state.each_with_index do |n, i|
      if n == 0
        return false if next_state[i] != 0 #0->1（右岸に移動）は出来ない
      else
        mover += 1 if next_state[i] != 1 #移動するものをカウント
      end
    end
    return false if mover > 2 #移動する物の数が2以上は不可能
  else #左から右へ
    state.each_with_index do |n, i|
      if n == 1
        return false if next_state[i] != 1 #1->0（左岸に移動）は出来ない
      else
        mover += 1 if next_state[i] != 0
      end
    end
    return false if mover > 2
  end
  true
end
def dejavu?(state)
  @history.each_with_index do |previous_string, i|
    previous = previous_string.split("").map{|a|a.to_i}
    p1 = previous[0,3].inject(:+)
    p2 = previous[3,3].inject(:+)
    s1 = state[0,3].inject(:+)
    s2 = state[3,3].inject(:+)
    return true if (p1 == s1) && (p2 == s2) && (i%2 == (@history.length)%2)
  end
  false
end

@history = []
@patterns = sieve(cases)
def _solve (state)
  @history << state.join("")
  return true if state.join("") == "111111"
    @patterns.each do |pattern|
      next_state = pattern.split("").map{|a|a.to_i}
      if possible?(state, next_state) && !dejavu?(next_state)
        return true if _solve(next_state) == true
      end
    end
  @history.pop
end

def solve_missionaries_cannibals(init)
  init_to_int = init.split("").map{|a|a.to_i}
  _solve(init_to_int)
  return @history
end

p solve_missionaries_cannibals("000000")

　
綺麗になってるぞー
負けないぞー