search

ms022019 / tic

0 stars 0 forks source link

[reviews] tokei.py #1

Open sumiredc opened 1 month ago

sumiredc commented 1 month ago

Comments

Static Badge

このアプリの実行方法を教えてもらえますか？

Static Badge

README.md ファイルが無いようなので、作成いただき手順が書いてあると良いと思いました https://docs.google.com/presentation/d/16hKTU2wf95HwukAXSIo1vpx1lp31kRPj7aSVag_HXZk/edit#slide=id.g2c9eed371c0_1_5

ms022019 commented 1 month ago

以下のコマンドで実行しています poetry run python .\tokei.py

sumiredc commented 1 month ago

@ms022019 ありがとうございます 色々とPC依存の設定があるようなので、動作している動画等などを連携いただけますと助かります

ms022019 commented 1 month ago

@sumiredc Analog Clock App 2024-05-18 20-57-04 (1).zip

ms022019 commented 1 month ago

動作動画となります。 こちらは、一応全体メールでは送らせていただいたつもりではいました。 Readmeについては、忘れていました。のちほど、こちらに提出します。 設定情報なく、お手数かけましたが、よろしくお願いいたします。

sumiredc commented 1 month ago

@ms022019 preview ありがとうございます！ メールで別途添付いただいていたんですね、それは失礼いたしました m(_ _)m

Readmeについては、忘れていました。のちほど、こちらに提出します。

こちらお待ちしております👍

tako-t-t-1046 commented 1 month ago

遅くなり申し訳ございません。 1.5期生の田中です。 課題の対応お疲れ様でした。 コードを確認させていただきましたので、気になった点などのコメントです。

Good Point

LGTM

ボタンやコンボボックスなどを用意いただいており、ユーザビリティを意識して対応いただいているのは良いですね。

また、それぞれの針に対しても色を変えるなど工夫いただいており、わかりやすいと思いました👍

コメントについて

nits

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L20-L21 summer_mode のコメントが ampm_mode のものになっていますので、修正いただきたいです。

コードのスタイルについて

nits

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L29-L32

math.pi/2 - i*(math.pi/6) など、「演算子の前後の半角スペースの有無」にばらつきがあるようです。 スタイルを合わせることも、コードの可読性につながるので、統一いただけると良いと思います。

三項演算子について

imo

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L35-L38

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L49

self.ampm_mode を利用した判定についてですが、 if ブロックを利用した判定や、三項演算子を利用した判定など、ばらつきがありそうです。（記述いただいたコードでは三項演算子が多い印象ですね）
24時間表記なのか？の分岐であれば、表現として統一しても良いかと感じました。

Debug 時のコードについて

nits

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L105-L110

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L115-L120

Debug のログや、ロジックのコメントアウトが残っているようです。
不要であれば削除をお願いします。

繰り返しの記載について

imo

https://github.com/ms022019/tic/blob/24daaeff7f7c5e270a084d3269b2a486ffad90a8/tokei.py#L130-L143

self.configure , self.clock.configure , self.clock.create_clock_face() と似た処理が並んでいますので、
共通化をさせることで、コードの見通しが良くなるかと思います。
細かい点ではありますが、積み重ねにより、可読性の高いコードとなります！

dark モードの表示について

want

image

dark モードにした際に、上側の時計が見えなくなってしまっています。
再描画の際に枠のカラーが変更できるとよいですね。

プログラムの依存関係について

want

別環境で動作させることも考慮し、プログラムの依存関係がわかるファイルがあるとよいですね。
Pythonだと、requirements.txt などがよく利用されますね。

全体的にClassで的確に分離されており、わかりやすいプログラムでした。
以上、よろしくお願いします。

ms022019 commented 1 month ago

@sumiredc poetry.zip pyproject.tomlとpoetry.lockをzipにして送ります。 こちらで、poetryの環境があれば、poetry installで環境構築できるかと思います。 時間おくれてすいませんでした。もし、なにか不都合ありましたら、おしらせいただけたらと思います

ms022019 commented 1 month ago

@tako-t-t-1046 @sumiredc 丁寧にコメントありがとうございました。 ３項演算子以外は修正して、commitしたつもりです。お知らせまで

直したところ 不必要なコメント、バグ 繰り返し処理を簡易化 色を変えてダークでも見やすく 演算子前の空白共通化 コメント間違い修正