[Groonga-commit] droonga/fluent-plugin-droonga at 1b22308 [master] Use SUB_FORMATTERS map to iterate over formatters

Back to archive index

Kouhei Sutou kou****@clear*****
Fri Dec 20 11:13:13 JST 2013


> +            method(sub_formatter_method_name).call(formatted_result)

これは、

  __send__(sub_formatter_method_name, formatted_result)

でいいですね!


ところで、SUB_FORMATTERSを使ってメタプログラミングチックなこ
とをするのはオーバースペックな気がしました。

今回のケースでは、こんな感じでcase whenで十分だと思いました。

  def format
    formatted_result = {}

    (@request.output || []).each do |name|
       case name
       when "count"
         value = format_count
       when "attributes"
         value = format_attributes
       when "records"
         value = format_records
       when "startTime"
         value = format_start_time
       when "elapsedTime"
         value = format_elapsed_time
       else
         next
       end
       formatted_result[name] = value
      end
    end

    formatted_result
  end

というのは、SUB_FORMATTERSを導入しても変更箇所を局所化できて
いないからです。新しくsub formatを追加したいときは

  * SUB_FORMATTERSに対応関係を追加
  * 違う場所でフォーマット用メソッドを定義

としなければいけません。

もし、sub formatterがメソッドじゃなくて、クラスになっていて

  class CountFormatter
    RsultFormatter::SUB_FORMATTERS["count"] = self

    def initialize(search_request, search_result)
      @request = search_request
      @result  = search_result
    end

    def format
      @result.count
    end
  end

  class AttributesFormatter
    RsultFormatter::SUB_FORMATTERS["attributes"] = self

    def initialize(search_request, search_result)
      @request = search_request
      @result  = search_result
    end

    def format
      # ...
    end
  end

  # ...

みたいに、自分のところで新しいsub formatを追加できて、

  def format
    formatted_result = {}

    (@request.output || []).each do |name|
       sub_formatter_class = SUB_FORMATTERS[name]
       next if sub_formatter_class.nil?
       sub_formatter = sub_formatter_class.new(@request, @result)
       formatted_result[name] = sub_formatter.format
      end
    end

    formatted_result
  end

という感じになっているなら変更箇所を局所化できていて
SUB_FORMATTERSを導入するメリットはある気がします。

ただ、sub formatはこれからモリモリ追加されていくようなもので
はない(例えば、ユーザーがプラグインで自由に追加できるもので
はない)ので、SUB_FORMATTERSを導入するのはオーバースペックに
感じました。(つらつらと並べても割にあいそう。)


In <1b2230843ca7306f6e4c9bfbc07aca2a4b3b8bca �� jenkins.clear-code.com>
  "[Groonga-commit] droonga/fluent-plugin-droonga �� 1b22308 [master] Use SUB_FORMATTERS map to iterate over formatters" on Thu, 19 Dec 2013 19:55:41 +0900,
  Yoji Shidara <null+groonga �� clear-code.com> wrote:

> Yoji Shidara	2013-12-19 19:55:41 +0900 (Thu, 19 Dec 2013)
> 
>   New Revision: 1b2230843ca7306f6e4c9bfbc07aca2a4b3b8bca
>   https://github.com/droonga/fluent-plugin-droonga/commit/1b2230843ca7306f6e4c9bfbc07aca2a4b3b8bca
> 
>   Message:
>     Use SUB_FORMATTERS map to iterate over formatters
> 
>   Modified files:
>     lib/droonga/searcher.rb
> 
>   Modified: lib/droonga/searcher.rb (+12 -18)
> ===================================================================
> --- lib/droonga/searcher.rb    2013-12-19 19:16:13 +0900 (8cd6782)
> +++ lib/droonga/searcher.rb    2013-12-19 19:55:41 +0900 (1a2f13a)
> @@ -325,6 +325,14 @@ module Droonga
>      end
>  
>      class ResultFormatter
> +      SUB_FORMATTERS = {
> +        "count"       => :format_count,
> +        "attribtues"  => :format_attributes,
> +        "records"     => :format_records,
> +        "startTime"   => :format_start_time,
> +        "elapsedTime" => :format_elapsed_time
> +      }
> +
>        class << self
>          def format(search_request, search_result)
>            new(search_request, search_result).format
> @@ -339,24 +347,10 @@ module Droonga
>        def format
>          formatted_result = {}
>  
> -        if need_element_output?("count")
> -          format_count(formatted_result)
> -        end
> -
> -        if need_element_output?("attributes")
> -          format_attributes(formatted_result)
> -        end
> -
> -        if need_element_output?("records")
> -          format_records(formatted_result)
> -        end
> -
> -        if need_element_output?("startTime")
> -          format_start_time(formatted_result)
> -        end
> -
> -        if need_element_output?("elapsedTime")
> -          format_elapsed_time(formatted_result)
> +        SUB_FORMATTERS.each do |name, sub_formatter_method_name|
> +          if need_element_output?(name)
> +            method(sub_formatter_method_name).call(formatted_result)
> +          end
>          end
>  
>          formatted_result




More information about the Groonga-commit mailing list
Back to archive index