Skip to content

Conversation

@chooyan-eng
Copy link

@chooyan-eng chooyan-eng commented Nov 28, 2017

lib/transport.rbへの修正

2行目のsplit(" ")を簡略化しました

APIドキュメントを見ると、splitはpetternをnilにする(省略する)と半角スペースで区切る挙動になります。

If pattern is nil, the value of $; is used. If $; is nil (which is the default), str is split on whitespace as if ` ‘ were specified.

そのため、2行目は

array = source.split("\n").map {|s| s.split()}

と書くことができ、さらにブロック内の処理が引数なしのメソッド呼び出しのみの場合は&:を使ってブロックを省略できるため、以下のように簡略化できます。

array = source.split("\n").map(&:split)

3~8行目の転置処理をmapメソッドを利用して修正しました

ここは元の行列を表す配列から新しい配列を作成する処理なので、mapを活用することでソースコードを簡略化することができます。結果を保持するための空の配列を定義する必要もなくなり、ソースコードがすっきりします。

変数名を読みやすく修正しました

このメソッドは行列を表す多次元配列を操作する処理ということで少し内容のイメージがしづらいため、なるべく変数名はその内容を表す単語にしておくことで読む側が楽になります。
行列は"matrix"、行は"row"、個々の値は"n"として名前を修正しています。

また、「転置」にあたる単語は"transport"ではなく"transpose"のようですので、ソースコード全体を通してこちらを使った方が良いでしょう。

転置行列 | Wikipedia

test/transport_test.rbへの修正

テストケースを分割しました

1つ目のテスト(3 x 3の行列)と2つ目のテスト(3 x 4の行列)では試験対象のデータが変わりますので、テストケースも分割しておいた方が良いでしょう。

@chooyan-eng chooyan-eng changed the title [@chooyan-eng] suggestions to make code better [chooyan-eng] suggestions to make code better Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant