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

最終日。

1週間リファクタなので、今日が最後になる。
多少CC値（サイクロマティック複雑度）は改善したものの、劇的な改善は見られず。
そもそもループでIF&IF&IFだから、地道にやるしかないにはないのかなーとは思うが。

if not handled and is_map and remaining_args:
      # The component is a dict or other key-value map; try to access a member.
      target = remaining_args[0]

      # Treat namedtuples as dicts when handling them as a map.
      if inspectutils.IsNamedTuple(component):
        component_dict = component._asdict()  # pytype: disable=attribute-error
      else:
        component_dict = component

      if target in component_dict:
        component = component_dict[target]
        handled = True
      elif target.replace('-', '_') in component_dict:
        component = component_dict[target.replace('-', '_')]
        handled = True
      else:
        # The target isn't present in the dict as a string key, but maybe it is
        # a key as another type.
        # TODO(dbieber): Consider alternatives for accessing non-string keys.
        for key, value in component_dict.items():
          if target == str(key):
            component = value
            handled = True
            break

      if handled:
        remaining_args = remaining_args[1:]
        filename = None
        lineno = None
        component_trace.AddAccessedProperty(
            component, target, [target], filename, lineno)
      else:
        error = FireError('Cannot find key:', target)
        candidate_errors.append((error, initial_args))

とおもったらこーゆー奴である。

２つ３つ対応して見えてきたこと。
・１つのIFのブロックで、handled componentその他
　多数が決定されている。
・それぞれ、ブロックで分かれていることは自明。メソッドに切り出せる
　範囲できっちり見えているが・・・複数の結果を返すメソッドをどうするか。で今の形になったのではないかと思われる。
・ひとまず、handled, component(last, trace, remaining_args,・・・この辺を固めてクラスにしてしまっても差し支えなさそう。
　それでいいのではか。モジュールの状態で扱ている関係で取り回しがしんどくなってそう。 思い切って直す。テストもあるんだ。やってみる。

あぁ。ぜんぜんむり。

クラス化をやっている途中で各変数の絡み具合が把握できなくて、つらくなってしまった。
このレベルで絡まれるとものすごく読むのがつらいんだなーとあらためて。複雑度「F」ランクっていうのは・・・
テスト通らないけどひゃっはーして、ひとまず各ブロックを丸ごとクラス化してしまおう。
どれぐらい改善するか。だけみてこのソースとはおさらばしよう。

3ブロックぐらいで504まで回復。ついにD判定になる。
のこりも頑張る

めちゃくちゃ突貫でつくってテストも通らないけど、
数値はだいぶ改善した（意味がない気がするが・・・）
とにかくメソッドをコンパクトにして、早期リターンを心がければだいぶ見通しがよくなる。
個人的もこれならとりあえず、各処理のブロックが明示されていい感じになったとおもっている。
ブロックそれぞれはやっぱり改善点はあるけど、もうちょっと踏み込んだ設計見直しをしないとつらそう。
componentとか。componentに持つべきっぽい処理がちらほらあるが、処理を見る限り型自体が不定っぽいので、悩ましい感じなのかもしれない。

# Copyright (C) 2018 Google Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

class CreateConponent():
  def __init__(self, component, initial_component, component_trace, args):
    self.component = component
    self.initial_component = initial_component
    self.component_trace = component_trace
    self.remaining_args = args
    self.Reflash()
    self.instance = None

  def GetRemaingArgs(self):
    return self.remaining_args

  def Reflash(self):
    self.handled = False
    self.initial_args = self.remaining_args
    self.last_component = self.component
    self.candidate_errors = []

  def _NeedCallAndUpdateTrace(self):
    """ Judge Need Execute _CallAndUpdateTrace"""
    if self.handled:
      return False

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

  def _KeepInstance(self):
    """ 
        RetuIf the initial component is a class, keep an instancerns
    """
    if not self.handled:
      return self.instance
    if not self.last_component is self.initial_component:
      return self.instance
    return self.component

  def KeepInstance(self):
    if not _NeedCallAndUpdateTrace():
      return
    # The component is a class or a routine; we'll try to initialize it or
    # call it.
    is_class = inspect.isclass(self.component)
    try:
      component, remaining_args = _CallAndUpdateTrace(
          self.component,
          self.remaining_args,
          self.component_trace,
          treatment='class' if is_class else 'routine',
          target=self.component.__name__)
      handled = True
    except FireError as error:
      self.candidate_errors.append((error, self.initial_args))

    # If the initial component is a class, keep an instance for use with -i.
    self.instance = self._KeepInstance()

  def IsTuple(self, is_sequence):
    if self.handled:
      return
    if not is_sequence:
      return
    if not self.remaining_args:
      return
    # The component is a tuple or list; we'll try to access a member.
    arg = self.remaining_args[0]
    try:
      index = int(arg)
      component = self.component[index]
      handled = True
    except (ValueError, IndexError):
      error = FireError(
          'Unable to index into component with argument:', arg)
      self.candidate_errors.append((error, self.initial_args))
    if handled:
      remaining_args = remaining_args[1:]
      filename = None
      lineno = None
      self.component_trace.AddAccessedProperty(
          component, index, [arg], filename, lineno)

  def IsDictOrOtherKeyValue(self, is_map):
    if self.handled:
      return
    if not is_map:
      return
    if not self.remaining_args:
      return

    # The component is a dict or other key-value map; try to access a member.
    target = self.remaining_args[0]

    # Treat namedtuples as dicts when handling them as a map.
    if inspectutils.IsNamedTuple(self.component):
      component_dict = self.component._asdict()  # pytype: disable=attribute-error
    else:
      component_dict = self.component

    if target in component_dict:
      component = component_dict[target]
      self.handled = True
    elif target.replace('-', '_') in component_dict:
      component = component_dict[target.replace('-', '_')]
      self.handled = True
    else:
      # The target isn't present in the dict as a string key, but maybe it is
      # a key as another type.
      # TODO(dbieber): Consider alternatives for accessing non-string keys.
      for key, value in component_dict.items():
        if target == str(key):
          self.component = value
          self.handled = True
          break
      if self.handled:
        self.remaining_args = self.remaining_args[1:]
        filename = None
        lineno = None
        self.component_trace.AddAccessedProperty(
            component, target, [target], filename, lineno)
      else:
        error = FireError('Cannot find key:', target)
        self.candidate_errors.append((error, self.initial_args))

  def IsHelpShortcut(self):
    """Determines if the user is trying to access help without '--' separator.

    For example, mycmd.py --help instead of mycmd.py -- --help.

    Args:
      component_trace: (FireTrace) The trace for the Fire command.
      remaining_args: List of remaining args that haven't been consumed yet.
    Returns:
      True if help is requested, False otherwise.
    """
    show_help = False
    if self.remaining_args:
      target = self.remaining_args[0]
      if target in ('-h', '--help'):
        # Check if --help would be consumed as a keyword argument, or is a member.
        component = self.component_trace.GetResult()
        if inspect.isclass(component) or inspect.isroutine(component):
          fn_spec = inspectutils.GetFullArgSpec(component)
          remaining_kwargs = _ParseKeywordArgs(self.remaining_args, fn_spec)
          show_help = target in remaining_kwargs
        else:
          members = dict(inspect.getmembers(component))
          show_help = target not in members

    if show_help:
      self.component_trace.show_help = True
      command = '{cmd} -- --help'.format(cmd=self.component_trace.GetCommand())
      print('INFO: Showing help with the command {cmd}.\n'.format(
          cmd=pipes.quote(command)), file=sys.stderr)

      self.remaining_args = []
    return show_help

  def TryObjectHandlerAccess(self):
    if not self.handled:
      return
    if not self.remaining_args:
      return

    # Object handler. We'll try to access a member of the component.
    try:
      target = self.remaining_args[0]
      self.component, consumed_args, self.remaining_args = _GetMember(self.component, self.remaining_args)
      self.handled = True

      filename, lineno = inspectutils.GetFileAndLine(self.component)

      self.component_trace.AddAccessedProperty(self.component, target, consumed_args, filename, lineno)

    except FireError as error:
      # Couldn't access member.
      self.candidate_errors.append((error, self.initial_args))

  def CallCallableObject(self, is_callable_object, used_separator, separator, saved_args):
    if not self.handled:
      return
    if not is_callable_object:
      return
    
    # The component is a callable object; we'll try to call it.
    try:
      component, remaining_args = _CallAndUpdateTrace(
              self.component,
              self.remaining_args,
              self.component_trace,
              treatment='callable')
      self.handled = True
    except FireError as error:
      self.candidate_errors.append((error, self.initial_args))

    if not self.handled and self.candidate_errors:
      error, self.initial_args = self.candidate_errors[0]
      self.component_trace.AddError(error, self.initial_args)

    if used_separator:
      # Add back in the arguments from after the separator.
      if self.remaining_args:
        self.remaining_args = self.remaining_args + [separator] + saved_args
      elif (inspect.isclass(self.last_component)
            or inspect.isroutine(self.last_component)):
        self.remaining_args = saved_args
        self.component_trace.AddSeparator()
      elif self.component is not self.last_component:
        self.remaining_args = [separator] + saved_args
      else:
        # It was an unnecessary separator.
        self.remaining_args = saved_args

  def NeedProgress(self):
    if not self.component is self.last_component:
      return True
    if not self.remaining_args == self.initial_args:
      return True
    # We're making no progress.
    return False

かわりにゴミ箱クラスができてしまったわけではある。
移動しただけとも言えなくはない。
・メソッド名を考えるタイミングで悩ましいところ、
・無駄に生存期間の長い変数
とかその辺を捕まえることはできたので、まーそれなりに効果はある。
これ、やったうえもう一回いちからキレイにしていけば、それなりに片付くと思う。
ということで python-Fireおわり（Fileだとおもってた。Fireだ）