ペアプロ実習 LRUHash

今回の Rails勉強会@東京(タグ「railstokyo」 を検索 - はてなブックマーク)では、ペアプロ、TDD(BDD)/RSpec 実習(指導 akasaka.rb) がありました。

お題は LUR な Hash っぽいもの。サイズ制限があったりしてそれを超えると古い物からなくなってくようなもの。

取り敢えずその時のソース

かなり問題ありと思うのですが取り敢えずその時のソースを掲げます。

スペック

require 'lruhash'
#$VERBOSE = true

describe LRUHash do
  before(:each) do
    @lruh = LRUHash.new(2)
    @var = nil
  end # before(:each) do
  
  it 'should new by new(size)' do
    lambda{ LRUHash.new }.should_not raise_error
    lambda{ LRUHash.new(5) }.should_not raise_error
    lambda{ LRUHash.new(5, nil) }.should raise_error ArgumentError
  end # it 'should new by new(size)' do
  
  it 'should be Hash like []=' do
    var = nil
    lambda{ var = LRUHash.new }.should_not raise_error
    lambda{ var[:key] = 'value' }.should_not raise_error
    var[:key].should == 'value'
  end # it 'should be Hash like []=' do
  
  it 'should be respond to size' do
    @lruh.size.should == 0
    @lruh[0] = 1
    @lruh.size.should == 1
    @lruh[1] = 2
    @lruh.size.should == 2
    @lruh[2] = 3
    @lruh.size.should == 2
    @lruh[3] = 4
    @lruh.size.should == 2
    @lruh[4] = 5
    @lruh.size.should == 2
  end # it 'should be respond to size' do
  
  it 'should be get and add some key-value s' do
    @lruh.size.should == 0
    @lruh[0] = 1
    @lruh.size.should == 1
    @lruh[1] = 2
    @lruh.size.should == 2
    @lruh[2] = 3
    @lruh.size.should == 2
    @lruh[0].should be_nil
    @lruh[1].should == 2
    @lruh[3] = 4
    @lruh.size.should == 2
    @lruh[2].should be_nil
  end # it 'should be get and add some key-value s' do
  
  it 'should be get and add some key-value s' do
    @lruh.size.should == 0
    @lruh[0] = 1
    @lruh.size.should == 1
    @lruh[1] = 2
    @lruh.size.should == 2
    @lruh.delete(1)
    @lruh.size.should == 1
    @lruh.instance_variable_get(:@updated).should == [0,1]
    @lruh.instance_variable_get(:@updated).should_not == [0]
    
    @lruh[2] = 3
    @lruh.instance_variable_get(:@updated).should == [0,2]
  end # it 'should be get and add some key-value s' do
end

実装

class LRUHash < Hash
  def initialize(max_size = 3)
    @max_size = max_size
    @updated = []
  end # def initialize(max_size = 3)
  
  alias :add []=
  def []=(key, value)
    #@updated.delete key
    @updated << key
    puts "u:#{@updated}, k+:#{self.keys + [key]}"
    @updated = @updated - (@updated - (self.keys + [key]))
    if self.size >= @max_size
      #self.keys.include? @updated[0]
      self.delete(@updated[0])
      @updated.shift
    end # if self.size > max_size
    p @updated if $VERBOSE
    add(key, value)
  end # def []=
  
  alias :get []
  def [](key)
    @updated.delete key
    @updated << key
    p @updated if $VERBOSE
    get key
  end # def [](key)
end # class LRUHash

ちょっと問題点

なんでメソッドエイリアスチェイン、そこは super でしょ

実装の事

  alias :add []=
  def []=(key, value)
    ＜云々＞

  alias :get []
  def [](key)
    ＜云々＞

Hashオープンクラスのメソッドを変更するんじゃなくて、ちゃんと継承 LRUHash < Hash してるんだから、そこは super でしょう。
何考えてたんだろう。

数値を使うのはセンス悪いよ

スペック

    @lruh.size.should == 0
    @lruh[0] = 1
    @lruh.size.should == 1
    @lruh[1] = 2
    @lruh.size.should == 2

「@lruh[0] = 1」とか。key と value で値をずらしてるのはなんか意図を感じるけど、ここで整数値を使うのはよくないでしょう、誤解を招きやすい(配列参照とか)し。key はシンボルにしようよ。値はまあ、数値でもいいから。

このシナリオっぽいスペックの書き方

スペック

  it 'should be get and add some key-value s' do
    @lruh.size.should == 0

    # 一つ登録
    @lruh[0] = 1
    @lruh.size.should == 1

    # 二つ登録
    @lruh[1] = 2
    @lruh.size.should == 2

    # 三つ登録しても
    @lruh[2] = 3
    # サイズは増えなくて
    @lruh.size.should == 2
    # 最初に登録したのが無くなってる
    @lruh[0].should be_nil
    # 二つ目に登録したのは残ってる、そんでアクセスして順番繰上げ
    @lruh[1].should == 2

    # 四つ登録して
    @lruh[3] = 4
    # サイズは増えなくて
    @lruh.size.should == 2
    # 三つ目に登録したのが無くなってる
    @lruh[2].should be_nil
  end # it 'should be get and add some key-value s' do

スペック

  it 'should be get, delete, and add some key-value s' do
    @lruh.size.should == 0
    # 一つ登録
    @lruh[0] = 1
    @lruh.size.should == 1
    # 二つ登録
    @lruh[1] = 2
    @lruh.size.should == 2

    # そこで二つ目に登録したのを削除
    @lruh.delete(1)
    @lruh.size.should == 1
    # でもキーリストからそのキーは消えてない
    @lruh.instance_variable_get(:@updated).should == [0,1]
    # 本当はキーリストはこうなるべきなんじゃないの、なってないけど
    @lruh.instance_variable_get(:@updated).should_not == [0]
    
    # また登録
    @lruh[2] = 3
    # 登録のときにお掃除してちゃんとなるのさ
    @lruh.instance_variable_get(:@updated).should == [0,2]
  end # it 'should be get and add some key-value s' do

順次状態を作りながら(変えながら)、should してくのも問題(一つの it に should は一つが原則っぽい、せめてなるたけ少なく)だが、コメントがないと何を確かめてるか分からないのはもっと問題。コメントを補えば良いというものでもない。it の引数文字列とか変数名メソッド名で自然と分かるようになってなくちゃ。

ちょっとだけ直す

今になって「かなり問題あり」という部分だけ修正したのがこちら

スペック

require 'lruhash'
#$VERBOSE = true

describe LRUHash do
  before(:each) do
    @lruh = LRUHash.new(2)
    @var = nil
  end # before(:each) do
  
  it 'should new by new(size)' do
    lambda{ LRUHash.new }.should_not raise_error
    lambda{ LRUHash.new(5) }.should_not raise_error
    lambda{ LRUHash.new(5, nil) }.should raise_error ArgumentError
  end # it 'should new by new(size)' do
  
  it 'should be Hash like []=' do
    var = nil
    lambda{ var = LRUHash.new }.should_not raise_error
    lambda{ var[:key] = 'value' }.should_not raise_error
    var[:key].should == 'value'
  end # it 'should be Hash like []=' do
  
  it 'should be respond to size' do
    @lruh.size.should == 0
    
    # 一つ登録
    @lruh[:a] = 'a'
    @lruh.size.should == 1
    
    # 二つ登録
    @lruh[:b] = 'b'
    @lruh.size.should == 2
    
    # 三つ登録しても
    @lruh[:c] = 'c'
    # サイズは最大値の儘
    @lruh.size.should == 2
    
    # 四つ登録しても
    @lruh[:d] = 'd'
    @lruh.size.should == 2
    # 五つ登録しても
    @lruh[:e] = 'e'
    @lruh.size.should == 2
  end # it 'should be respond to size' do
  
  it 'should be get and add some key-value s' do
    @lruh.size.should == 0
    @lruh[:a] = 'a'
    @lruh.size.should == 1
    @lruh[:b] = 'b'
    @lruh.size.should == 2
    @lruh[:c] = 'c'
    @lruh.size.should == 2
    # 順に三つ登録すると
    
    # 一つ目に登録したのはもうない
    @lruh[:a].should be_nil
    # 二つ目に登録したのはある、そしてアクセスして順序繰り上げ
    @lruh[:b].should == 'b'
    
    # 四つ登録
    @lruh[:d] = 'd'
    @lruh.size.should == 2
    # さっき繰り上げといたので、二つ目は残ってる
    @lruh[:b].should_not be_nil
    # 三つ目が消えてる
    @lruh[:c].should be_nil
  end # it 'should be get and add some key-value s' do
  
  it 'should be get and add some key-value s' do
    @lruh.size.should == 0
    @lruh[:a] = 'a'
    @lruh.size.should == 1
    @lruh[:b] = 'b'
    @lruh.size.should == 2
    # 順に三つ登録すると
    
    # 二つ目に登録したのを削除
    @lruh.delete(:b)
    @lruh.size.should == 1
    # でもキーリストからそのキーは消えてない
    @lruh.instance_variable_get(:@updated).should == [:a, :b]
    # 本当はキーリストはこうなるべきなんじゃないの、なってないけど
    @lruh.instance_variable_get(:@updated).should_not == [:a]
    
    # また登録
    @lruh[:c] = 'c'
    # 登録のときにお掃除してちゃんとなるのさ
    @lruh.instance_variable_get(:@updated).should == [:a, :c]
  end # it 'should be get and add some key-value s' do
end

実装

class LRUHash < Hash
  def initialize(max_size = 3)
    @max_size = max_size
    @updated = []
  end # def initialize(max_size = 3)
  
  def []=(key, value)
    @updated.delete key
    @updated << key
    puts "u:#{@updated}, k+:#{self.keys + [key]}"
    @updated = @updated - (@updated - (self.keys + [key]))
    if self.size >= @max_size
      #self.keys.include? @updated[0]
      self.delete(@updated[0])
      @updated.shift
    end # if self.size > max_size
    p @updated if $VERBOSE
    super
  end # def []=
  
  def [](key)
    @updated.delete key
    @updated << key
    p @updated if $VERBOSE
    super
  end # def [](key)
end # class LRUHash

個別のコメントや当日の感想、更に直すべき所とか、また後で追記します(/6/22 24:40)。

個別のコメントの前に列の伸び方、方向

(/6/23 追記)
みなアクセス順にキーを貯える配列を使っていた。t-wadaさんのヒントもあって、1.9系でその必要のないペアを除いて、みんなアクセス順をみるキーの配列を作っていた。
それでレビューの時にちょっと疑問が出たのできいてみたわけです。

「みんな新しいキーを配列の後ろから追加して、古いのを前から削除してる。なぜ」

はじめ偶然じゃないか、たまたまみんなそうだっただけだよという声でしたが、良くきいてみると感覚的にみんなそうしてるみたい。

配列の操作はまず shift、push、pop の組みで考えてるから、とか。待ちキューのつもりだから新しいのは列の後ろに追加してって、先頭で順次処理してく、処理が今の場合削除なだけ、とか。後ろから消すって pop ってことだけど、pop はスタック処理で縦に積んでるという感覚で、今回の列に並べる感じに合わない、とか。
なんか感覚的にそんな感じというのがあって面白かった。

僕自身は、それらの感覚ももっともなんだけど、列というのが後ろの方に行くにつれてよく分かんなくなっていって、後ろの末で忘れられてなくなってくというイメージもあって、後ろに伸びてくようにするか前にずれてくようにするか、実はちょっと随分迷うところあります。
そして、Array に要素追加するのに「<<」が一番楽だという実装感覚で前にずれてくように作ります、作りました。
僕の個人的な感覚です。

NetBeans とか IDE

(/6/27追記)
ペアプログラミングとしては、はじめパートナーの NetBeans 環境でやってみた。RSpec は初めてということで、そのインストールから。で、IDE は IDEなりに便利。それで RSpec(付き)プロジェクト作れるようになってるし、少し作るとスペックファイルまで自動生成してくれる、before(:esch)ブロックで取り敢えずテスト用のインスタンス変数 new する所まで。
ただ、require のパスが通らなかったり、始めちょっとそれなり設定必要みたいで、そこで時間かけても仕方ないのでその辺で環境こちらのものに変えました。

IDE は IDE なりに良いですね。リードに従ってると凄く楽っぽい。

(/6/27 なんかばたばたしてるうちに一週間経ってしまった。もう個別のコメントはなしで、ソース見て下さい。)