調べ物した結果

現役SEが仕事と直接関係ないことを調べた結果とか感想とか

Python-Fileのソースを眺める③~適当リファクタ~

前回と同じ要領で。

とりあえず、分岐の複雑なIF文をどんどんメソッドに切り出していこう。
なるべくならメソッド名をきちんと考えたいが・・・

handled ??? not handled???

このへん。目がちかちかする。カオス。

    if not handled and is_callable:
      # The component is a class or a routine; we'll try to initialize it or
      # call it.
      is_class = inspect.isclass(component)

      try:
        component, remaining_args = _CallAndUpdateTrace(
            component,
            remaining_args,
            component_trace,
            treatment='class' if is_class else 'routine',
            target=component.__name__)
        handled = True
      except FireError as error:
        candidate_errors.append((error, initial_args))

      if handled and last_component is initial_component:
        # If the initial component is a class, keep an instance for use with -i.
        instance = component

    if not handled and is_sequence and remaining_args:

読み進めていく限り、not handledの判定が頻発する。
かくブロックの中で handledが決定されていれば、処理をしないということか?
handledがTrueになる条件を探っていく。

    handled = False
    candidate_errors = []

    is_callable = inspect.isclass(component) or inspect.isroutine(component)
    is_callable_object = callable(component) and not is_callable
    is_sequence = isinstance(component, (list, tuple))
    is_map = isinstance(component, dict) or inspectutils.IsNamedTuple(component)

    if not handled and is_callable:

うーん。whileのなかでhandledがfalseで初期化されてるから、
最初に実行されるこのifは not handledの判定が常に trueなんじゃ・・・
そうだけど、あえて書くことで似たような塊ということを明示しているのか?はて。

なかなにややこしいが、is_callableの判定( isCalssなのか、isroutineなのか)

    is_callable = inspect.isclass(component) or inspect.isroutine(component)
||
はここでしか使っていない。内部に閉じ込めても問題なさそう。思い切ってこのブロックごとメソッドに切り出してみるか

>|python|
  def _is_class_or_routine(handled, is_callable, component, remaining_args, component_trace, candidate_errors, last_component, initial_component, initial_args):
    """ check the component is a class or a routine.
    Returns:
      try to initialize it or call it result. 
    """
    if handled:
      return handled
    if not is_callable:
      return handled
    
    # The component is a class or a routine; we'll try to initialize it or
    # call it.
    is_class = inspect.isclass(component)
    try:
      component, remaining_args = _CallAndUpdateTrace(
          component,
          remaining_args,
          component_trace,
          treatment='class' if is_class else 'routine',
          target=component.__name__)
      handled = True
    except FireError as error:
      candidate_errors.append((error, initial_args))

    if handled and last_component is initial_component:
      # If the initial component is a class, keep an instance for use with -i.
      instance = component

途中までかいた。これ切り出してもだめだわ。引数おおいしなぁ・・・
セット処理も死ぬほどあるなぁ。困ったぞ。おかしいかもしれんが、それでも切り出してみるか。
pythonってinterface持ち込めるのかな・・・
componentが判定すべきことだとおもうんだよな。この辺。

テスト通してみると failed..いかんね。
デバッグでとめてみると、handled だとか、instanceだとか、
メソッドの内部で変更したはずの値が変わっていないようす。おそらくimmutable関連の問題だろうなぁ。
https://python.ms/object/identity/immutable/#%E7%B0%A1%E5%8D%98%E3%81%AB%E8%A8%80%E3%81%88%E3%81%B0
を見る。
うーん。boolもimmutableのようだから、これは意図した動作できてないなぁ。
メソッドに切り出す作戦は問題だということだな。C#のノリで行けるとおもってかいてしまった
とにかくこのブロックでやることが多すぎるのが問題か。もう少し分解できないか探る。

    is_callable = inspect.isclass(component) or inspect.isroutine(component)
    is_callable_object = callable(component) and not is_callable
    is_sequence = isinstance(component, (list, tuple))
    is_map = isinstance(component, dict) or inspectutils.IsNamedTuple(component)

    if not handled and is_callable:

is_callable はcomponentが定まればよさそう。component自体にで持っていてほしいが、
性質よりそもそもまともなinstanceかどうかも怪しい。ので、まずはここだけ抜き出す。

def _NeedCallAndUpdateTrace(handled, component):
  """ Judge Need Execute _CallAndUpdateTrace"""
  if handled:
    return False

  if inspect.isclass(component):
    return True
  if inspect.isroutine(component):
    return True
  
  return False

シンプル判定。

    is_callable = inspect.isclass(component) or inspect.isroutine(component)
    is_callable_object = callable(component) and not is_callable
    is_sequence = isinstance(component, (list, tuple))
    is_map = isinstance(component, dict) or inspectutils.IsNamedTuple(component)

    if _NeedCallAndUpdateTrace(handled, component):

いったん、条件はすっきりしたと思う。
is_callable の判定は余剰になったけど、どうかな。いったんこれでもいい気がするが。
中の方も分解できないか。見てみる。

      if handled and last_component is initial_component:
        # If the initial component is a class, keep an instance for use with -i.
        instance = component

ここ。これも切り出そう

def _KeepInstance(handled, instance, component, last_component, initial_component):
  """ 
      RetuIf the initial component is a class, keep an instancerns
  """
  if not handled:
    return instance
  if not last_component is initial_component:
    return instance
  return component
      # If the initial component is a class, keep an instance for use with -i.
      instance = _KeepInstance(handled, instance, component, last_component, initial_component)

微妙かなぁ。テスト流してサイクロマティック複雑度が改善するか見てみよう。
コマンドは「radon cc D:\git_refact\python-fire -n C 」
f:id:couraeg:20191029220445p:plain
前回が「376」で今回が「400」なので効果はあるよう。
やはりとりあえずガード節ねじ込みまくれば数値は改善する。そらそうなんだけど。

もう少しコンパクトになってくれば、別の改善も見えてくるだろう。このまま進めよう。