Skip to content

プログラムの修正(リバーシ編)課題の提出#1

Open
yokomaru wants to merge 3 commits into
mainfrom
my_bug_reversi
Open

プログラムの修正(リバーシ編)課題の提出#1
yokomaru wants to merge 3 commits into
mainfrom
my_bug_reversi

Conversation

@yokomaru

Copy link
Copy Markdown
Owner

概要

対応

必要修正要件

エラーの起きているテストを修正

  • test_put_stone
  • test_cannot_put_stone
  • test_turn
  • test_finished_of_full_board
  • test_finished_of_player_skip_board

実行結果

  • 以下を実行
ruby test/reversi_methods_test.rb
  • 実行結果スクショ
スクリーンショット 2024-02-25 21 16 13

@JunichiIto JunichiIto left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

コメントしました!

Comment thread lib/reversi_methods.rb Outdated
def turn(board, target_pos, attack_stone_color, direction)
return false if target_pos.out_of_board?
return false if target_pos.stone_color(board) == attack_stone_color
return false if target_pos.stone_color(board) == attack_stone_color || target_pos.stone_color(board) == BLANK_CELL

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

||でまとめるなら、target_pos.out_of_board?|| でまとめてもいいのでは?と思いました。
コードの一貫性の観点でどういう修正が望ましいか検討してみてください〜。

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.

コメントありがとうございます!

コードの一貫性の観点

なるほど・・!target_pos.out_of_board?target_pos.stone_color(board)に関する分岐は分けた方がいいのかな?と思って上記のようにしたのですが、確かに一貫性がないように見えます。。!
違和感なく読めるコードを目指して行きます!

コメントいただいた箇所は以下のようにif文でまとめて修正しました!

if target_pos.out_of_board? || target_pos.stone_color(board) == attack_stone_color || target_pos.stone_color(board) == BLANK_CELL
  return false
end

@JunichiIto JunichiIto left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1点コメントしましたが大きな問題ではないのでこれでOKです!🙆‍♂️

Comment thread lib/reversi_methods.rb
def turn(board, target_pos, attack_stone_color, direction)
return false if target_pos.out_of_board?
return false if target_pos.stone_color(board) == attack_stone_color || target_pos.stone_color(board) == BLANK_CELL
if target_pos.out_of_board? || target_pos.stone_color(board) == attack_stone_color || target_pos.stone_color(board) == BLANK_CELL

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

横に並べる方法もありますが、横に長いコードはそれはそれで可読性が悪いので、一貫して縦に並べる、というアプローチもあります。(好みの問題ですが)

    return false if target_pos.out_of_board?
    return false if target_pos.stone_color(board) == attack_stone_color
    return false if target_pos.stone_color(board) == BLANK_CELL

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.

2 participants