YOSHINO日記

プログラミングに関すること

ActiveStorageでみた不自然な条件分岐に関して

解せぬ。

ActiveStorageのコミットログを最初から見て勉強しております。

  def self.decode(encoded_key)
    key, expires_at = verifier.verified(encoded_key)

    if key # ここ!!
      key if expires_at.nil? || Time.now.utc < expires_at
    end
  end

keyがnilを来る場合を考えても、条件分岐は使わずに、

key if expires_at.nil? || Time.now.utc < expires_at

だけでいいのではないかと思う。

確かに、if のところでnil判定したほうが、expires_atの式の評価とかしなくて良くなるから、 速くなるとは思うけど、なんか汚い感じした。

そんな理由であっているんですかね。解せぬ。

なるほど

その後のコミットで以下のように修正されていました。

encodeメソッドもついでにのせてしまった。

  class << self

    def encode(key, expires_in: nil)
      verifier.generate([ key, expires_at(expires_in) ])
    end

    def decode(encoded_key)
      key, expires_at = verifier.verified(encoded_key)
      key if key && fresh?(expires_at)
    end

    private
      def expires_at(expires_in)
        expires_in ? Time.now.utc.advance(seconds: expires_in) : nil
      end

      def fresh?(expires_at)
        expires_at.nil? || Time.now.utc < expires_at
      end
  end

不自然な条件分岐は消えて

key if key && fresh?(expires_at)

このようになっています。

これだったらキレイです。モヤモヤが取れました。不自然な条件分岐があるのなら、それは、関数として切り出しても良いサインなのかもしれない。