Skip to content
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

add 二次方程式 #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

EdamAme-x
Copy link

前回のPRにて指摘された内容を参考にし、修正。
API等の型定義は、H-gotoさんの
リファクタリングにより解決していただきました。

@H-goto16
Copy link
Contributor

こんにちは。
props.d.ts内には、propsの型のみを定義したほうが良いと思います。
新たにtypesディレクトリ下にimport.d.tsなどの型定義ファイルを新たに作成する方がわかりやすい思いました。

@EdamAme-x
Copy link
Author

了解
ご意見ありがとうございます。
可読性についての意見助かります。

pages/quadratic-equation.tsx Outdated Show resolved Hide resolved

const filteredAnswers = parsedIntAnswers.filter(answer => answer !== null) as number[]

if (answers.some(answer => filteredAnswers.includes(answer))) {
Copy link
Member

Choose a reason for hiding this comment

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

どちらか片方の解が入力されていれば正解と判定されますが、想定通りの挙動でしょうか?

(仕様的に結構な割合で0を入力すると正解になってしまうのはかんたんすぎるような気もして💭)

output

Copy link
Author

Choose a reason for hiding this comment

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

順序を無視して採点する為に どちらかが正解なら通すようにしてしまってますね…
他の採点方法に切り替えます

Copy link
Member

Choose a reason for hiding this comment

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

解を求めよ
</MathJax.Provider>
<div>
<input className={styles.textbox} placeholder="x1 , x2" type="text" value={ans} onChange={(e) => { setAns(e.target.value) }} inputMode="numeric"/>
Copy link
Member

@yoidea yoidea Jul 11, 2023

Choose a reason for hiding this comment

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

どうやらiPadの分割キーボートではカンマの入力ができないようでした。
iPadからの利用をメインに考えているので記号は含まない[0-9]+形式で解答できるのが理想的です!
(要件定義がしっかりできていなくてすみません🙇‍♂️)

IMG_0660

Copy link
Author

Choose a reason for hiding this comment

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

なるほど、、
実装します。

@EdamAme-x
Copy link
Author

修正点
-,/をボタンから入力可能に
解答欄は二つに分け、順序は自動的にソート。
二つの解が一致してればokに。
また 定数が0になる確率を調整出来るように。
現在定数が0になる確率と 他の整数になる確率は同程度です。

@rossy0213
Copy link
Member

@EdamAme-x
お疲れ様です。ラムダと一緒に運営してるロッシーです。
複数な解への対応及びiPadのキーボード仕様に対する解決案のUIを実装して頂き、ありがとうございます。

ラムダのコメントに基づいて、こちらのPRを以下のように変更してもらうことは可能でしょうか?

  • 入力がどちらの解が正解なら正解と判定する仕様で、0が解に含まれる確率を落とす。(現状の体験を維持する。)
  • 以前の1つだけ入力ボックスに戻し、解は全て正の整数になるようにする。(マイナス、分数用のボタンを削除)
    理由としては、UIのシンプルさをキープしたいからです。

また、今回実装した分は素晴らしいものでそのまま捨てるのはもったいないと思っています。
提案として、達人モードみたいなボタンを用意して、

  • 1つの解でも正解すれば正解と判定する。 (通常、デフォルトモード)
  • 2つの解両方正解すれば正解と判定する。また、マイナスと分数も対応する。(達人モード)
    このように、対応して頂くことは可能でしょうか? (対応して頂ける場合、達人モードは新しいPRでお願いします。)

@EdamAme-x
Copy link
Author

なるほど。
0が解に含まれる確率の変更は可能です。
2つ目ですが 1つ目の解は実装可能として
解が1つ合ってればokだとすれば可能です。

そろそろ寝るので申し訳無いです。
明日学校の隙間の時間で 時間があれば実装してみます。

@rossy0213
Copy link
Member

ありがとうございます。
OSSなので、時間取れる時で問題ないです。 🙇

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.

4 participants