-
Notifications
You must be signed in to change notification settings - Fork 13
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
kgpMainでの異常系処理を追加 #802
kgpMainでの異常系処理を追加 #802
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
レビューしました.細かい点をいくつかコメントしたので修正/検討お願いします.
|
||
@Test | ||
public void test04() { | ||
assertEquals(-9, new CloseToZero().close_to_zero(-10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertEquals(-9, new CloseToZero().close_to_zero(-10)); | |
assertEquals(-9, new CloseToZero().close_to_zero(-8)); |
じゃないでしょうか
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertEquals
は expect, actual の順で書くのでこれで正しい.
Assertj#assertThat
は(英語文法とassertjの仕組み上)actual, expect の順で書くので混同しがち.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すいません,その通りですね
混乱してました
|
||
@Test | ||
public void test04() { | ||
assertEquals(-9, new CloseToZero().close_to_zero(-10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同様
@@ -17,8 +18,12 @@ | |||
* @deprecated | |||
*/ | |||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コンストラクタが必要であれば @deprecated
は削除した方が良いと思います
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
至った過程をメモしておく.
このクラスはsingletonにする予定,かつ別のクラスが参照していたので,tech debtのつもりで @Depre にしてあった.
でもこのクラスはある失敗を表すクラスで,そもそもsingletonであるべきではない.失敗の理由を記載できなくなる.
そのためdepreするべきではない.
@@ -8,6 +9,8 @@ | |||
import org.junit.Rule; | |||
import org.junit.Test; | |||
import org.junit.rules.TemporaryFolder; | |||
import ch.qos.logback.classic.Level; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unusedなので除去してください
このPRには関係ないですが,↓の import jp.kusumotolab.kgenprog.ga.variant.Variant;
もunusedなのでついでに除去をお願いします
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます.良いと思います
マージしますね
お疲れ様でした
resolve #610
resolve #645
resolve #734
resolve #740
kGenProgMainの異常系処理を追加.
利用者目線での利便性が向上.
やったこと
本質
上に付随
その他リファクタリング系
無駄なRuntimeException宣言を削除
無駄なTestExecutor#finish #initializeを削除
Mainクラスでのログ出力周りをinner class化,さらに改善