Codebreaker解いてみました #2
Conversation
| def guess(numbers) | ||
| @numbers = numbers.each_char.to_a | ||
|
|
||
| '+' * hit_count + '-' * match_count |
There was a problem hiding this comment.
hit_count の処理が２回実行されることになりますね。
There was a problem hiding this comment.
メモイズって初めて聞きました。
そういう実装があるんですねー。勉強になります
There was a problem hiding this comment.
メモイズするとgessメソッド中で初期化処理書かないといけない気がするので、
あまり処理速度気にする必要がなければあえてメモイズしないのもアリじゃないでしょうか？
There was a problem hiding this comment.
現状 readable ですしね。
僕だと、↓ みたいな感じで書いちゃいますけど、readable じゃなくなるんですよね。。。
'+' * (h = hit_count) + '-' * (total_count - h)There was a problem hiding this comment.
なるほど、文字列の上書きができればそんな感じのやり方でもうちょっとうまく書ける気がしてきました
There was a problem hiding this comment.
こんな感じに書けばメソッド呼び出し1回ずつでいけそうです
('+' * total_count).each_char.to_a.tap {|marks|
marks.fill('-', hit_count..-1)
}.join
うーん、でもやっぱり読みにくいですね。。。
There was a problem hiding this comment.
('+' * total_count).each_char.to_aは、↓ で良さそうです。
total_count.times.map { '+' }この方法も面白いですね。
There was a problem hiding this comment.
なるほど、ちょっとすっきりしますね
total_count.times.map { '+' }.tap {|marks|
marks.fill('-', hit_count..-1)
}.join
文字列を一旦配列にしてうんぬんが無駄な気がしますね
文字列の一部を直接指定した文字で埋められたら良さそうですが。。。
('+' * total_count).fill('-', hit_count..-1)
みたいなのが理想です
|
|
||
| def guess(numbers) | ||
| @numbers = numbers.each_char.to_a | ||
| @hit_count = nil |
There was a problem hiding this comment.
この初期化は不要です。インスタンスメソッド存在しなくてもnilが返るので hit_count の中でいきなり書けばOKです。
There was a problem hiding this comment.
2回目gessるとバグりませんか?
ちょっとテスト書いてみます
There was a problem hiding this comment.
おお、ほんとですね。すみません。
そうするとメモイズがだいぶダサいので最初のコードが美しいと思います。
| def total_count | ||
| (0..9).map(&:to_s).reduce(0) do |count, n| | ||
| count + [@secret.count(n), @numbers.count(n)].min | ||
| end |
There was a problem hiding this comment.
頭ひねってこのやり方しか出てこなかったです
どう実装するのが自然でしょうかね。。。
配列のメソッドがまだイマイチうまく使いこなせません。。。