Kouhei Sutou
kou****@clear*****
Fri Jan 16 11:54:35 JST 2015
> むしろクラス変数にしてもいいくらいなんじゃないかなあと思っていました。 うーん、それはやり過ぎかなぁ。 > テストのしやすさを考えると、必須のパラメータということにしておくのが無難 > でしょうか。 というようにテストのしやすさがありますし。 ClusterにはEngineStateを渡すのはどうですかねぇ。そいつはloop もNodeMetadataも持っていますし、なにより、「Engine自身のこと」 を渡したい(やりたいことはこれですよね?)というのを実現する ためにEngineStateを渡すというのは理にかなっていると思います。 EngineStateはEngine自身のことを管理しているオブジェクトだっ たと思うので。 In <54B8696B.4060904 �� clear-code.com> "Re: [Groonga-commit] droonga/droonga-engine �� 16702db [master] Reuse the instance of NodeMetadata globally" on Fri, 16 Jan 2015 10:29:15 +0900, YUKI Hiroshi <yuki �� clear-code.com> wrote: > むしろクラス変数にしてもいいくらいなんじゃないかなあと思っていました。 > (全体で1つの「現在のそのノード自身のrole」をリアルタイムに把握したい、 > というニーズなので) > > テストのしやすさを考えると、必須のパラメータということにしておくのが無難 > でしょうか。 > > Kouhei Sutou wrote: >>> Reuse the instance of NodeMetadata globally >> >> 任意(引数で渡されてもいいし、渡されなくてもいい)なのか >> 必須(必ず引数で渡されること)なのか明確にしませんか? >> >> 必須だけどもしかしたら誰かが渡し忘れるかもしれない、というの >> で >> >>> + @sender_node_metadata ||= NodeMetadata.new >> >> みたいなコードを入れているなら呼び出す側を修正する方がよいと >> 思います。そっちの方がいろんな場所に↑みたいな「もしものとき」 >> のためのコードが入り込まなくて全体としてスッキリすると思いま >> す。 >> >> あと、必須なら >> >>> + def initialize(loop, options={}) >> ... >>> + @node_metadata = options[:metadata] >> >> みたいに「options」という名前で受け取るのではなく、 >> initialize(loop, metadata)みたいに受け取るようにしたほうが意 >> 図が明確になります。 >> >> >> 個人的にはプロセス全体で「Reuse」したいなら必須にした方がよ >> いと思います。 >> >> In <16702db325bfed21ef5cfd206abfe7f834ece203 �� jenkins.clear-code.com> >> "[Groonga-commit] droonga/droonga-engine �� 16702db [master] Reuse the instance of NodeMetadata globally" on Thu, 15 Jan 2015 16:23:31 +0900, >> YUKI Hiroshi <null+groonga �� clear-code.com> wrote: >> >>> YUKI Hiroshi 2015-01-15 16:23:31 +0900 (Thu, 15 Jan 2015) >>> >>> New Revision: 16702db325bfed21ef5cfd206abfe7f834ece203 >>> https://github.com/droonga/droonga-engine/commit/16702db325bfed21ef5cfd206abfe7f834ece203 >>> >>> Message: >>> Reuse the instance of NodeMetadata globally >>> >>> Modified files: >>> lib/droonga/cluster.rb >>> lib/droonga/engine.rb >>> lib/droonga/engine_node.rb >>> lib/droonga/engine_state.rb >>> >>> Modified: lib/droonga/cluster.rb (+6 -2) >>> =================================================================== >>> --- lib/droonga/cluster.rb 2015-01-15 16:06:55 +0900 (c3b2a40) >>> +++ lib/droonga/cluster.rb 2015-01-15 16:23:31 +0900 (80490b9) >>> @@ -26,12 +26,13 @@ module Droonga >>> attr_accessor :catalog >>> attr_writer :on_change >>> >>> - def initialize(loop) >>> + def initialize(loop, options={}) >>> @loop = loop >>> >>> @catalog = nil >>> @state = nil >>> @on_change = nil >>> + @node_metadata = options[:metadata] >>> >>> reload >>> end >>> @@ -157,7 +158,10 @@ module Droonga >>> def create_engine_nodes >>> all_node_names.collect do |name| >>> node_state = @state[name] || {} >>> - EngineNode.new(name, node_state, node_metadata.role, @loop) >>> + EngineNode.new(name, >>> + node_state, >>> + @loop, >>> + :metadata => node_metadata) >>> end >>> end >>> >>> >>> Modified: lib/droonga/engine.rb (+5 -2) >>> =================================================================== >>> --- lib/droonga/engine.rb 2015-01-15 16:06:55 +0900 (67a838f) >>> +++ lib/droonga/engine.rb 2015-01-15 16:23:31 +0900 (1d5298e) >>> @@ -32,8 +32,11 @@ module Droonga >>> >>> attr_writer :on_ready >>> def initialize(loop, name, internal_name) >>> - @state = EngineState.new(loop, name, internal_name) >>> - @cluster = Cluster.new(loop) >>> + @state = EngineState.new(loop, name, >>> + internal_name, >>> + :metadata => node_metadata) >>> + @cluster = Cluster.new(loop, >>> + :metadata => node_metadata) >>> @catalog = load_catalog >>> @state.catalog =****@clust***** = @catalog >>> @dispatcher = create_dispatcher >>> >>> Modified: lib/droonga/engine_node.rb (+12 -4) >>> =================================================================== >>> --- lib/droonga/engine_node.rb 2015-01-15 16:06:55 +0900 (ac2194f) >>> +++ lib/droonga/engine_node.rb 2015-01-15 16:23:31 +0900 (9157b28) >>> @@ -23,10 +23,10 @@ module Droonga >>> >>> attr_reader :name >>> >>> - def initialize(name, state, sender_role, loop) >>> + def initialize(name, state, loop, options={}) >>> @name = name >>> @state = state >>> - @sender_role = sender_role >>> + @sender_node_metadata ||= options[:metadata] >>> >>> parsed_name = parse_node_name(@name) >>> @sender = FluentMessageSender.new(loop, >>> @@ -54,11 +54,11 @@ module Droonga >>> >>> def forwardable? >>> return false unless live? >>> - role == @sender_role >>> + role == sender_role >>> end >>> >>> def writable? >>> - case @sender_role >>> + case sender_role >>> when NodeMetadata::Role::SERVICE_PROVIDER >>> true >>> when NodeMetadata::Role::ABSORB_SOURCE >>> @@ -133,6 +133,14 @@ module Droonga >>> role == NodeMetadata::Role::ABSORB_DESTINATION >>> end >>> >>> + def sender_role >>> + sender_node_metadata.role >>> + end >>> + >>> + def sender_node_metadata >>> + @sender_node_metadata ||= NodeMetadata.new >>> + end >>> + >>> def output(message, destination) >>> command = destination["type"] >>> receiver = destination["to"] >>> >>> Modified: lib/droonga/engine_state.rb (+2 -1) >>> =================================================================== >>> --- lib/droonga/engine_state.rb 2015-01-15 16:06:55 +0900 (bb21f23) >>> +++ lib/droonga/engine_state.rb 2015-01-15 16:23:31 +0900 (bf0eb31) >>> @@ -36,7 +36,7 @@ module Droonga >>> attr_accessor :catalog >>> attr_accessor :on_finish >>> >>> - def initialize(loop, name, internal_name) >>> + def initialize(loop, name, internal_name, options={}) >>> @loop = loop >>> @name = name >>> @internal_name = internal_name >>> @@ -47,6 +47,7 @@ module Droonga >>> @on_ready = nil >>> @on_finish = nil >>> @catalog = nil >>> + @node_metadata = options[:metadata] >>> end >>> >>> def start >> >> _______________________________________________ >> Groonga-commit mailing list >> Groonga-commit �� lists.sourceforge.jp >> http://lists.sourceforge.jp/mailman/listinfo/groonga-commit >> > > -- > 結城 洋志 <YUKI Hiroshi> > E-mail: yuki �� clear-code.com > > 株式会社クリアコード > 〒170-0005 東京都豊島区南大塚3-29-9 > 中野ビル3階 > TEL : 03-5927-9440 > FAX : 03-5927-9441 > WWW : http://www.clear-code.com/ > > _______________________________________________ > Groonga-commit mailing list > Groonga-commit �� lists.sourceforge.jp > http://lists.sourceforge.jp/mailman/listinfo/groonga-commit