0
0

More than 1 year has passed since last update.

Railsにおける配列への代入とリファクタリング

Posted at

問題のルール

  1. lengthとwidthが引数で渡され、最大サイズの正方形を可能な限り求めて、その正方形の一辺の長さを配列にまとめて戻り値にする。
  2. lengthとwidthが同じ長さの場合はnilを返す
    イメージ図は下記で、
    image.png
    テストとしては下記のようなものになる
  sqInRect(5, 3) should return [3, 2, 1, 1]
  sqInRect(3, 5) should return [3, 2, 1, 1]

最初に書いたコード

ざっと以下のように書いたが、毎度ながら空の配列listの定義が少し見にくい時がある。

  def check_smallest_square(line_a, line_b, list)
    ## sortの代わりにminmaxでもよかった
    short_line, long_line = [line_a ,line_b].sort

    if short_line == long_line
      nil
    else
      list << short_line
    ## long_line - short_lineを引数にすると長くなって気持ち悪いと思ったが、変数名の長さがそもそも良くなかった。変に1行使って処理書く方が醜い
      long_line -= short_line
      check_smallest_square(long_line, short_line, list) || list << short_line
    end
  end

  def sqInRect(line_a, line_b)
    return nil if line_a == line_b

    ## スクリプトの癖でいつも空の配列を定義してそこに値を入れようとしてる。
    list = Array.new
    check_smallest_square(line_a, line_b, list)
  end

空の配列をリファクタリングしてみた

[] + []は一つの配列になることを利用して、書いてみたが直感的にわかりやすいのか?? 変数名とif文をもう少し綺麗に

def check_smallest_square(line_a, line_b)
  short_line, long_line = [line_a ,line_b].sort

  if short_line == long_line
    nil
  else
    [short_line] + (check_smallest_square(long_line - short_line, short_line) || [short_line])
  end
end

def sqInRect(line_a, line_b)
  return nil if line_a == line_b
  check_smallest_square(line_a, line_b)
end

変数名とif文を綺麗にする

min,maxは予約語みたいなイメージがあるから少し気持ち悪いが文字が少なくてみやすい。
メソッドを分ける意味すらなさそう。

def find_square(lng, wdth)
  min, max = [lng ,wdth].sort
  min == max ? nil : [min] + (find_square(max - min, min) || [min])
end

def sqInRect(lng, wdth)
  find_square(lng, wdth)
end
0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0