New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor `transport` method by @cheezenaan #22

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@cheezenaan
Copy link

cheezenaan commented Dec 6, 2017

pull requestのdescriptionにはタケシが納得するように自分の変更点の意図を説明してあげてください。

という README.md の説明をすっ飛ばして直接コメント入れてしまいました><

@cheezenaan
Copy link
Author

cheezenaan left a comment

ざっとレビューしますた🙇

assert_equal output, transport(input)
class Transpose < MiniTest::Test
def test_transpose
input = <<~EOS

This comment has been minimized.

@cheezenaan

cheezenaan Dec 6, 2017

Author

[IMO]
「ドキュメントとしてのテストコード」という観点をもって書いてみると「より読みやすくできるのではないか…🤔」と考えを深められるかもしれません。

[IMO]
Ruby 2.4(2.3?) 系からはヒアドキュメントに <~~EOS が追加されたので、これを使ってみるといいと思います。
ref. https://docs.ruby-lang.org/ja/latest/doc/spec=2fliteral.html#here

# たとえば、こんなかんじです
text = <~~EOS
  劇場版 響け!ユーフォニアム
  〜北宇治高校吹奏楽部へようこそ〜
EOS
def transpose(source)
source_array = source.split("\n").map(&:split)
transposed_array = source_array.transpose

This comment has been minimized.

@cheezenaan

cheezenaan Dec 6, 2017

Author

[IMO]
自分がやりたいと思っていることは、たいてい他の誰かもやりたいと思っています。
その実現方法がたとえば gem として公開されていたり、 Ruby 標準のライブラリとして実装されていることが多いです。

今回タケシさんがやりたいことは Array#transpose として実装されているので、こちらを使ってみましょう。
ref. https://ref.xaio.jp/ruby/classes/array/transpose

@cheezenaan cheezenaan changed the title [WIP] Refactor `transport` method Refactor `transport` method by @cheezenaan Dec 6, 2017

cheezenaan added some commits Dec 6, 2017

Refactor `transpose`
- Rename variables
- Use `&` operator

@cheezenaan cheezenaan force-pushed the cheezenaan:refactor_array_transport branch from 7342222 to aaa9e30 Dec 6, 2017

@cheezenaan
Copy link
Author

cheezenaan left a comment

補足コメントです

def transpose(source)
source_array = source.split("\n").map(&:split)

This comment has been minimized.

@cheezenaan

cheezenaan Dec 6, 2017

Author

[IMO]
&演算子を使うと、ブロックを使った処理をもう少しシュッと書けるようになります😄
ref. https://qiita.com/kasei-san/items/0392097791d3a5998216

# これら2つの結果は同じです :D
irb(main):018:0> (1..10).map { |n| n.to_s }
=> ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10"]
irb(main):017:0> (1..10).map(&:to_s)
=> ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10"]

@cheezenaan cheezenaan closed this Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment