Skip to content

配列の章の演習問題の修正#465

Merged
aster-void merged 30 commits into
masterfrom
462-fix-array-exercise
Oct 18, 2023
Merged

配列の章の演習問題の修正#465
aster-void merged 30 commits into
masterfrom
462-fix-array-exercise

Conversation

@aster-void
Copy link
Copy Markdown
Contributor

@aster-void aster-void linked an issue Oct 14, 2023 that may be closed by this pull request
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Oct 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9d9ae44
Status: ✅  Deploy successful!
Preview URL: https://ba6091d5.utcode-learn.pages.dev
Branch Preview URL: https://462-fix-array-exercise.utcode-learn.pages.dev

View logs

Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/_samples/array-max/script.js Outdated
Comment thread docs/1-trial-session/10-array/_samples/array-sum-for-of/script.js Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
@chvmvd
Copy link
Copy Markdown
Contributor

chvmvd commented Oct 15, 2023

追加でコミットメッセージは、しっかりとつけてほしいです...
Gitが変になった時に、直すのがつらくなるので...
一つの機能に対して一つのコミットが適切なコミットメッセージとともにあると何かが合った時に直すのが楽。

Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Copy link
Copy Markdown
Contributor

@chelproc chelproc left a comment

Choose a reason for hiding this comment

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

  • オブジェクトを説明する前にオブジェクトと言ってませんか?
  • 配列自体が値であることを説明したほうがいいですね。スライドにあったような図もあるとより良いです。
  • document.writeを関数の中と外に置く例もあるといいですね
  • 「モジュール化」自体は重要な概念ではあるものの、モジュール化することは関数化することとは違いますし、一定のまとまりにコードを分割することを「モジュール化」以外の言い方で表すこともあります。「モジュール化前/後」という表記はモジュール化の方法が1通りしかないような誤解を与える危険がありますし、「分割する単位が1つの関心であるべきである」という重要な情報が伝わらない可能性がある気がします。このセクションはちょっと見直したほうがいい気がします。


:::danger
配列の長さにかかわらず配列の最初の値を使うような処理をする場合は、長さが0である空の配列を渡された時に例外処理することを忘れないでください
配列の長さにかかわらず配列の最初の値を使うような処理をする場合は、長さが0である空の配列を除外することを忘れないでください
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

このエクスクラメーションマークどうでしょう?

Comment thread docs/1-trial-session/08-loop/index.md
@chvmvd
Copy link
Copy Markdown
Contributor

chvmvd commented Oct 16, 2023

ここらへんは別PRで対応お願いします!

@chvmvd
Copy link
Copy Markdown
Contributor

chvmvd commented Oct 16, 2023

準備できたら、レビューをリリクエストとかしてお知らせください。

@aster-void aster-void requested review from chelproc and chvmvd October 18, 2023 00:32
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
Comment thread docs/1-trial-session/10-array/index.md Outdated
パーツに分割すると、次のようなメリットがあります。

モジュール化前:
- ブロックあたりのコードが短くなるので、読みやすい
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

コードを短くできるというのは、少し違う気がするかも。単純に関心を分離できることが重要で、コードを短くすることが大切なわけではない気がしてる。コードは短いのがよいということはないはず。

:::

## モジュール化
## パーツに分割する
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここを変える必要はない気がする?

モジュール化前:
- ブロックあたりのコードが短くなるので、読みやすい
- パーツごとにテストができるので、デバッグがしやすい
- 汎用性のあるパーツなら、使いまわしができる
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

汎用性のないものはそもそもあるのかなあ。汎用性のないものを想定させると、ただ無関係のものを凝集させただけのコードを書くのを助長しそう。

@chvmvd
Copy link
Copy Markdown
Contributor

chvmvd commented Oct 18, 2023

モジュール化のセクションは、一旦今のままにして練習問題の方をマージしたいです...
練習問題の方をマージした状態ではやく見直したい...

@chvmvd
Copy link
Copy Markdown
Contributor

chvmvd commented Oct 18, 2023

  • オブジェクトを説明する前にオブジェクトと言ってませんか?

    • 配列自体が値であることを説明したほうがいいですね。スライドにあったような図もあるとより良いです。

    • document.writeを関数の中と外に置く例もあるといいですね

    • 「モジュール化」自体は重要な概念ではあるものの、モジュール化することは関数化することとは違いますし、一定のまとまりにコードを分割することを「モジュール化」以外の言い方で表すこともあります。「モジュール化前/後」という表記はモジュール化の方法が1通りしかないような誤解を与える危険がありますし、「分割する単位が1つの関心であるべきである」という重要な情報が伝わらない可能性がある気がします。このセクションはちょっと見直したほうがいい気がします。

このプルリクエストに関係のないことは別のところに書いてほしいです...(いつまでもマージできなくなる...)

@aster-void aster-void requested a review from chvmvd October 18, 2023 11:17
Copy link
Copy Markdown
Contributor

@chvmvd chvmvd left a comment

Choose a reason for hiding this comment

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

一旦これでマージしちゃいましょう!!
モジュール化については、別PRで要検討ってことで。

@aster-void
Copy link
Copy Markdown
Contributor Author

マージします!モジュール化はIssueにでもします

@aster-void aster-void merged commit 9f10cb8 into master Oct 18, 2023
@aster-void aster-void deleted the 462-fix-array-exercise branch October 18, 2023 12:19
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