Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

名前を先頭一致で検索できるようにした #11

Merged
merged 2 commits into from
Mar 10, 2014

Conversation

yuuna
Copy link
Contributor

@yuuna yuuna commented Mar 9, 2014

東京、京都などの都道府県のsuffixがない状態で検索したかったので、検索ロジックを修正しました。
検索オーダーがhas_value?に加えて増えていますが、最大47^2なので問題ないということで……

なお、京都→京都府、東京→東京都を実現するために、先頭一致にしています

@chocoby
Copy link
Owner

chocoby commented Mar 9, 2014

PR ありがとうございます。
東京 などで検索できるのは便利ですね。確認します! 🙋

@chocoby
Copy link
Owner

chocoby commented Mar 9, 2014

確認しました。バッチリだと思います!

元々実装されていた検索ロジックが微妙だったのでこの機会に書きなおしてみました。
問題なさそうであればこちらの PR をマージした後に反映しようと思います(この PR で書き換えていただいても構いません)。

結果:

# 北海道
       user     system      total        real
bench1  0.310000   0.000000   0.310000 (  0.310929)
bench2  0.000000   0.000000   0.000000 (  0.001119)

# マッチなし
       user     system      total        real
bench1  0.330000   0.000000   0.330000 (  0.327541)
bench2  0.020000   0.000000   0.020000 (  0.017841)

コード:

            Benchmark.bm do |x|
              x.report('bench1') {
                1000.times {
                  self.bench1(args[:name])
                }
              }
              x.report('bench2') {
                1000.times {
                  self.bench2(args[:name])
                }
              }
            end

bench2 が新しいロジックです。
:name, :name_e と指定しているため、新しい項目が入った場合、変更に弱いのですが、まあ滅多に無いと思います。

    def self.bench1(name)
      result = Mapping.data.select { |_, v|
        v if v.values.map{|v2| v2 !~ /^#{name.downcase}/ }.delete_if{|i| i == true}.length > 0
      }.first

      return if result.nil?

      result[0]
    end

    def self.bench2(name)
      name = name.downcase

      Mapping.data.each do |m|
        if m[1][:name].start_with?(name) || m[1][:name_e].start_with?(name)
          return m[0]
        end
      end

      nil
    end

また、テストも追加しました。
想定ケースとしては以下のものでよいでしょうか?

      describe '都道府県名(先頭一致)' do
        let(:pref) { JpPrefecture::Prefecture.find(name: '東京') }
        it { expect(pref.name).to eq '東京都' }

        context 'マッチする都道府県が複数あった場合' do
          let(:pref) { JpPrefecture::Prefecture.find(name: '宮') }
          it { expect(pref.name).to eq '宮城県' }
        end

        context 'マッチする都道府県が複数あった場合(英語表記)' do
          let(:pref) { JpPrefecture::Prefecture.find(name: 'miya') }
          it { expect(pref.name_e).to eq 'Miyagi' }
        end
      end

@yuuna
Copy link
Contributor Author

yuuna commented Mar 9, 2014

ロジックですが、さらに効率のいい方法をもいついて実装しなおしました。
このpull requestでmergeしてみてください

テストケースでは、"東京"が東京、"京都"が京都になれば完璧だとおもいます。

あとKernel methodである"p"がoverrideされていたので、ついでに直しておきました。

@chocoby
Copy link
Owner

chocoby commented Mar 10, 2014

あとKernel methodである"p"がoverrideされていたので、ついでに直しておきました。

確かにそうですね...。ありがとうございます 🙇

それではマージします。後はこちらで手を加えて本日 14-15 時ぐらいに Gem をアップデートします。
今しばらくお待ちください 🙆‍♀️

chocoby added a commit that referenced this pull request Mar 10, 2014
名前を先頭一致で検索できるようにした
@chocoby chocoby merged commit d17a880 into chocoby:master Mar 10, 2014
@chocoby
Copy link
Owner

chocoby commented Mar 10, 2014

v0.6.0 をリリースしました。
https://rubygems.org/gems/jp_prefecture/versions/0.6.0

ありがとうございました!:smile:

@yuuna
Copy link
Contributor Author

yuuna commented Mar 11, 2014

@chocoby

ありがとうございます~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants