Skip to content

fizzbuzzプログラムを作成#1

Open
hiro23900 wants to merge 3 commits into
mainfrom
fizzbuzz
Open

fizzbuzzプログラムを作成#1
hiro23900 wants to merge 3 commits into
mainfrom
fizzbuzz

Conversation

@hiro23900

Copy link
Copy Markdown
Owner

No description provided.

Comment thread 01.fizzbuzz/fizzbuzz.rb Outdated
@@ -0,0 +1,7 @@
(1..20).each { |n|

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubyのeachにブロックを渡すとき、複数行の場合は{}ではなくdo...endを使うのが慣例です。

(1..20).each do |n|
  # ...
end

1行で書く場合は{}、複数行の場合はdo...endという使い分けを覚えておくと良いです。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

承知いたしました。修正いたしました。

Comment thread 01.fizzbuzz/fizzbuzz.rb Outdated
@@ -0,0 +1,7 @@
(1..20).each { |n|
(puts "FizzBuzz"; next) if n.modulo(3 * 5) == 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

行末に余分なスペースが入っています。エディタの設定で「保存時に行末スペースを削除する」をオンにしておくと、こういったものを自動で防げます。

また、n.modulo(3 * 5)n.modulo(15) と書けますし、% 演算子を使って n % 15 == 0 と書くこともできます。どちらがより読みやすいか意識してみてください。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • vimの設定で、行末のスペースを削除する設定を.vimrcにいたしました。
  • 確かにmoduloメソッドは直観的にわかりにくいので、%に変更しました。ただ、(15)よりも(3 * 5)とした方が、意味合いがわかると思いましたのでとりあえず残しております。はやり、(15)の方が適しているということでしたら修正いたします。

Comment thread 01.fizzbuzz/fizzbuzz.rb Outdated
@@ -0,0 +1,7 @@
(1..20).each { |n|
(puts "FizzBuzz"; next) if n.modulo(3 * 5) == 0
(puts "Buzz"; next) if n.modulo(5) == 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2行目と同様、行末に余分なスペースが入っています。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • == の前にスペースが2個あったので不要なスペース1個を削除しました。行末にはスペースはありませんでした。
  • moduloメソッドを%に書き替えました。確かにこちらの方がわかりやすいです。

Comment thread 01.fizzbuzz/fizzbuzz.rb Outdated
(puts "FizzBuzz"; next) if n.modulo(3 * 5) == 0
(puts "Buzz"; next) if n.modulo(5) == 0
(puts "Fizz"; next) if n.modulo(3) == 0
puts n

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

行末に余分なスペースが入っています。こちらも合わせて取り除いてください。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 行末のスペースを削除しました。

@hiro23900

Copy link
Copy Markdown
Owner Author

@pjord2026 レビュー有難うございました。
下記のように修正いたしました。

  • 1行目
    • 指摘されてないですが、n では不親切と思い、numに変更しました。
    • 複数行のブロックなので、[]ではなく、do ~ end を使用しています。
  • 2行目
    • 行末のスペースを削除しました。
    • moduloメソッドを%に書き替えました。確かにこちらの方がわかりやすいです。
    • pjordさんから、3 * 5 よりも15の方が良いのではとのアドバイスがありましたが、意味合いがわかるように今回は「3 * 5」をそのまま残しました。
  • 3行目
    • == の前にスペースが2個あったので不要なスペース1個を削除しました。
    • moduloメソッドを%に書き替えました。確かにこちらの方がわかりやすいです。
  • 4行目
    • == の前にスペースが2個あったので不要なスペース1個を削除しました。
    • moduloメソッドを%に書き替えました。確かにこちらの方がわかりやすいです。
  • 5行目
    • 行末のスペースを削除しました。
  • 6行目
    • 複数行のブロックなので、[]ではなく、do ~ end を使用しました。

@pjord2026 ご確認をよろしくお願いします。

@hiro23900

Copy link
Copy Markdown
Owner Author

if/elsif/else/end を利用してコードを見やすくなるよう変更いたしました。
コードレビューをよろしくお願いします。

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.

3 participants