-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
feat(ajaxapp): Promiseを活用する を追加 #116
Conversation
が認められてないのか |
ESLint 3.2.0のバグ(間違って入ったBreaking Change)ですね |
### エラーハンドリング | ||
|
||
このままではPromiseに置き換えた意味がないので、エラーハンドリングを行いましょう。 | ||
Promiseのコンテキスト内で発生したエラーは、`Promise.catch`を使って一箇所で受け取れます。 |
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.
Promise.catch
Promise#catch ? static methodっぽく見える
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.
#110
表記についてのIssue
|
@azu 指摘された点を直してみたので、もう一度レビューしてもらってもいいでしょうか |
そこで、HTML文字列を組み立てる`createView`関数とHTMLを表示する`displayView`関数を作り、 | ||
処理を分割します。 | ||
そこで、HTML文字列を組み立てる`createView`関数とHTMLを表示する`displayView`関数を作り、処理を分割します。 | ||
さらに、エントリポイントとして新しく`main`関数を作り、`getUserInfo`関数を呼び出すようにします。 |
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.
やや唐突なので、「なぜエントリーポイントが必要なの?」って前触れか、後述する〜のためとかがあると良さそうな気がします。
(エントリーポイントって何?はさすがになくて大丈夫か)
@@ -143,14 +148,21 @@ function getUserInfo(userId) { | |||
} | |||
``` | |||
|
|||
### Promiseチェーンへの置き換え | |||
### thenチェーンへの置き換え |
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.
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.
すごく微妙な感じですがPromiseチェーンのままでよかったっぽいですね…
771a447
to
2d4849f
Compare
|
||
Promiseチェーンを使って処理を分割する利点は、同期処理と非同期処理を区別せずに連鎖できることです。 | ||
ひとつの処理として書いてしまうと、非同期処理の結果を同期的に扱うことは難しくなります。 | ||
そういう場合に`then`で区切っておけば、前の処理が非同期処理であっても、その値はコールバック関数の引数として同期的に扱えます。 |
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.
その値はコールバック関数の引数として同期的に扱えます。
この辺の同期処理と非同期処理って言葉が何か違和感がある。
実際には結局非同期で動いてるはずなので、"その値はコールバック関数の引数として同期的に扱えます"というのは何を言ってるのかがよくわからない感じがします。
これは"コードを見た目上直列に書くことができるので同期的に書いているかのように扱える" ってことだと思うけど、それは"同期的に扱える"なのかが違和感がある感じ。
(この部分が省かれてる or 非同期がまるで同期処理として動くような感じに読める違和感がある)
async/awaitとかならもっと直列的な感じだけど、実際に処理を待つ(await)ので、処理が同期的であるワケではない気がします。
"区別せずに"ってフレーズを既に使ってるので、それに寄せていくのがいい気がする。
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.
「同期処理のように書くことができる」っていう表現がよさそうですね
ひとつの処理として書いてしまうと、非同期処理の結果を同期的に扱うことは難しくなります。 | ||
そういう場合に`then`で区切っておけば、前の処理が非同期処理であっても、その値はコールバック関数の引数として同期的に扱えます。 | ||
ひとつの処理として書いてしまうと、非同期処理の結果を扱うためにコードは複雑になります。 | ||
そういう場合に`then`で区切っておけば、前の処理が非同期処理であっても、値を引数として受け取るコールバック関数はまるで同期処理のように書くことができます。 |
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.
少し大げさな感じがするので、
- はまるで
+ を
or
+ は
ぐらいでいい気がします。
そういう場合に`then`で区切っておけば、前の処理が非同期処理であっても、値を引数として受け取るコールバック関数はまるで同期処理のように書くことができます。 | ||
それぞれの関数はどんな型の値を受け取り、どんな型の値を返すかだけに注意すればよいので、処理が簡潔になりコードの見通しがよくなるでしょう。 | ||
一般に、同期的に書かれた処理を後から非同期処理へと変更することは、全体を書き換える必要があるため難しいです。 | ||
そのため、最初から処理を分けておき、それを`then`を使って繋ぐことで、変更に強いコードを書くことができます。 |
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.
- それを`then`を使って繋ぐことで
+ 処理を`then`で繋ぐことで
とかかな。
代名詞を無くしたかった感じ
LGTM. 後はページとして見た時に違和感ないかをチェックして直していくといいと思います。 |
レビューありがとうございました。すごく助かりました。 |
#9