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

fix: 「クラス」における誤字修正や説明の改善 #1598

Merged
merged 4 commits into from
Feb 18, 2023

Conversation

morinokami
Copy link
Contributor

「クラス」を読んでいて見つけた誤りや文の乱れなどを修正しました。

Comment on lines 152 to 153
また、コンストラクタは初期化処理を書く場所であるため、`return`文で値を返すべきではありません。
JavaScriptでは、コンストラクタ関数が任意のオブジェクトを返すことが可能ですが、行うべきではありません。
Copy link
Contributor Author

Choose a reason for hiding this comment

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

この箇所について、完全に同じではありませんが似たようなことが続いて書かれており不自然に感じたため、後半の文を削除しました。

Copy link
Collaborator

@azu azu Feb 18, 2023

Choose a reason for hiding this comment

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

微妙に異なることを言っていて、どちらかという後者の方がストレートな表現な気がします。
前者は、仕様的にできないようにも読めます。コードの対応を考えると後者をベースにしたほうが良い気がしました

また、コンストラクタ関数は`return`文で任意のオブジェクトを返すことが可能ですが、行うべきではありません。
なぜなら、クラスを`new`演算子で呼び出し、その評価結果はクラスのインスタンスを期待するのが一般的であるためです。

// 現在要素数より小さな`newLength`が指定された場合、指定した要素数となるように末尾を削除する
if (newLength < currentItemLength) {
// 現在要素数より小さな`newLength`が指定された場合、指定した要素数となるように末尾を削除する
Copy link
Contributor Author

Choose a reason for hiding this comment

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

直下の else if ブロックに合わせ、コメント位置を修正しました。

Copy link
Collaborator

Choose a reason for hiding this comment

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

これは元の位置にあるのが通常な気がします。
コメントに条件の説明が入ってるので、条件より後に説明があるのは不自然に見えます。

@bot-user
Copy link

bot-user commented Feb 17, 2023

Deploy Preview for js-primer ready!

Name Link
🔨 Latest commit 227c14b
🔍 Latest deploy log https://app.netlify.com/sites/js-primer/deploys/63f0f8d80256df00082499cb
😎 Deploy Preview https://deploy-preview-1598--js-primer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

// 現在要素数より小さな`newLength`が指定された場合、指定した要素数となるように末尾を削除する
if (newLength < currentItemLength) {
// 現在要素数より小さな`newLength`が指定された場合、指定した要素数となるように末尾を削除する
Copy link
Collaborator

Choose a reason for hiding this comment

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

これは元の位置にあるのが通常な気がします。
コメントに条件の説明が入ってるので、条件より後に説明があるのは不自然に見えます。

source/basic/class/README.md Outdated Show resolved Hide resolved
Comment on lines 152 to 153
また、コンストラクタは初期化処理を書く場所であるため、`return`文で値を返すべきではありません。
JavaScriptでは、コンストラクタ関数が任意のオブジェクトを返すことが可能ですが、行うべきではありません。
Copy link
Collaborator

@azu azu Feb 18, 2023

Choose a reason for hiding this comment

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

微妙に異なることを言っていて、どちらかという後者の方がストレートな表現な気がします。
前者は、仕様的にできないようにも読めます。コードの対応を考えると後者をベースにしたほうが良い気がしました

また、コンストラクタ関数は`return`文で任意のオブジェクトを返すことが可能ですが、行うべきではありません。
なぜなら、クラスを`new`演算子で呼び出し、その評価結果はクラスのインスタンスを期待するのが一般的であるためです。

morinokami and others added 2 commits February 18, 2023 23:04
Co-authored-by: azu <azu@users.noreply.github.com>
@morinokami morinokami requested a review from azu February 18, 2023 14:11
@@ -986,7 +985,7 @@ example.instanceMethod();
example.prototypeMethod();
```

しかしこの2つのメソッドの定義方法は、メソッドを定義先となるオブジェクトが実際に異なります
しかしこの2つのメソッドの定義方法は、メソッドの定義先となるオブジェクトが実際は異なります
Copy link
Collaborator

@azu azu Feb 18, 2023

Choose a reason for hiding this comment

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

この"実際に"は、強調的な意味合いだと思うので、"実際は"にすると少し不自然な気がしました。(どっちも"実際には"の短縮のように感じるかも)

📝 何に対して 実際は なのかが気になる。
何かを否定する文な気がするけど、否定する対象の文がない。
(同じオブジェクトに定義しているように見えます とかの前提がない)

source/basic/class/README.md Outdated Show resolved Hide resolved
Co-authored-by: azu <azu@users.noreply.github.com>
@morinokami morinokami requested a review from azu February 18, 2023 16:16
@azu azu added Type: Errata 誤記、誤字、表記揺れ Type: Need to Publish Publishに反映したほうがよいもの labels Feb 18, 2023
@azu azu merged commit 454f019 into asciidwango:master Feb 18, 2023
@azu
Copy link
Collaborator

azu commented Feb 18, 2023

ありがとうございました

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Errata 誤記、誤字、表記揺れ Type: Need to Publish Publishに反映したほうがよいもの
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants