回答編集履歴

追記部分への対応

2018/12/15 04:55

投稿

miyabi-sun

スコア21158

test CHANGED Viewed

@@ -175,3 +175,255 @@
 }
 ```
+---
+【追記箇所への回答】
+> 1.staticなメソッドをasyncさせる方法はないのでしょうか？
+まぁ、参考書とかもないですし情報少なくて辛いですよね。
+これを参考にしてみてください。
+※await構文の右辺がPromiseの場合、実行してresolve(value)のvalue部分を取り出しますが、String等の値であればそれをそのまま利用しようとします。
+```JavaScript
+class Animal {
+  static async meow () {
+    return await 'meow!'
+  }
+}
+Animal.meow().then(console.log);
+// "meow!"が出力される
+```
+> 2.以下のvalidateRegisterUsersData()のようなPromiseチェーンを組むときに3変数になってしまうのですが、isFilledUsernameの引数をusernameだけにしてもPromiseチェーンは組めるのでしょうか？
+できる or できないで言えばできます。
+Promiseのreslve関数を実行するときの書式で、`resolve(1, 2, 3)`という風に3つの引数で実行した場合、
+`.then((a, b, c) => console.log(a, b, c))`という風に3つの引数で受けることが可能です。
+ただし、async / await構文で戻って来る場合、resolveの第一引数しか持って帰れません。
+Promise返す関数は最小の挙動を記述すれば、そんなにたくさんの引数を持って帰りたいという要求は出るはずがありません。
+> 3.以下の2点を守ってコードを書き換える際に、Promiseチェーンを組むときは以下のような形になると思ったのですが、間違ってますか？
+そもそもの話で、Promiseが必要な関数/メソッドってどういうものだと思いますか？
+それは「非同期処理であるべきもの」だけです。
+非同期処理であるべきものとは何でしょうか？
+それは主に「HDDに問い合わせる」「ネットワーク越しに問い合わせる」といった、メモリに保存済みのものを呼び出すものより圧倒的に遅いものがその対象になるべきです。
+> ```JavaScript
+> static __isFilledPassword(password, isValidated, errors) {
+>   new Promise((resolve, reject) => {
+>     isValidated = false;
+>     errors.password = "passwordが未入力です。";
+>   });
+> }
+> ```
+passwordが変数が空か否かを確認するメソッドは非同期処理であるべきか？
+と聞かれれば、別にHDDやネットワーク越しに問い合わせるわけではないので不要です。
+要するにお前Promiseである必要ないじゃんって話ですし、
+isFilledPasswordというメソッド/関数ならば、普通のエンジニアはtrue or falseが帰って来ることを想定するはずです。
+この辺のメソッド名の名付けや挙動は英語力も必要になってきます。
+Authenticatorクラスの中の話なので、例えばinvalidReasonsという配列が帰ってきそうなメソッド名にしてはいかが？
+配列が空、つまり`reasons.length === 0`がtrueならば妥当なデータ、falseならば不正な値というわけです。
+これなら死ぬほど使いやすくなったと思いませんか？
+```JavaScript
+class Authenticator {
+  static invalidReasons({username, password}) {
+    const reasons = [];
+    if (username === '') {
+      reasons.push('usernameが未入力です');
+    }
+    if (password === '') {
+      reasons.push('passwordが未入力です。');
+    }
+    return reasons;
+  }
+}
+```
+> `isValidated = false;`
+細かいですが、こういう使い方はバグの原因なので極力やめてください。
+こういう使い方する関数を「副作用がある関数」と呼び、一定以上の技量のエンジニアには嫌われる使い方です。
+パフォーマンス改善のテクニックの一つではあるのですが、これを意識せず使いまくったシステムだと単純にバグの量が100倍増えるでしょう、それだけのコストを払う意味があるケースは実際殆ど存在せず禁じ手になっています。
+また、今回の用途では正しく動作しません。
+何故かというとString、Number、Booleanなどのプリミティブ値は値渡しなので、関数の内側で引数として宣言したisValidated変数の内容は変わりますが、引数として渡した外側の世界の変数には影響しません。
+```JavaScript
+var add3 = it => {
+    it += 3;
+    return it;
+}
+var a = 1;
+console.log(add3(a)); // 4
+console.log(a); // 1
+```
+オブジェクトや配列等のオブジェクト系はJavaScriptでは一見参照渡しのように思えますが、
+実際には参照の値渡しなので、やはり束縛が切れるので無意味です。
+```JavaScript
+var nameChange = (obj, name) => {
+  obj = {name};
+  return obj;
+}
+var taro = {name: 'taro'}
+var jiro = nameChange(taro, 'jiro')
+console.log(taro); // {name: 'taro'}
+console.log(jiro); // {name: 'jiro'}
+// 副作用を起こしたければこうする
+var nameChange2 = (obj, name) => {
+  obj.name = name; // 変数への代入ではなく、プロパティを書き換えるような挙動にする
+  return obj;
+}
+var taro = {name: 'taro'}
+var jiro = nameChange2(taro, 'jiro')
+console.log(taro); // {name: 'jiro'} <- 副作用の再現に成功
+console.log(jiro); // {name: 'jiro'}
+```
+> // __isFilledUsername = username, isValidated, errors => new Promise() だとエラー
+ES2015のクラス構文の書き方を復習しましょう
+[クラス - MDN](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Classes)
+それと、アロー関数のルールで`username, isValidated, errors => new Promise()`のように引数が複数になる場合は引数の部分のカッコは省略できません。
+アロー関数でカッコを省略できるのは引数が1個のケースだけです。
+[アロー関数 - MDN](https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Functions/Arrow_functions)

多分これで完成形

2018/12/15 04:55

投稿

miyabi-sun

スコア21158

test CHANGED Viewed

@@ -120,7 +120,7 @@
 // 関数は外のbodyみたいな値に依存するのではなく、引数で受け付けたほうがいい
-const isUniqueUsername = (username) => new Promise((resolve, reject) => {
+const isUniqueUsername = username => new Promise((resolve, reject) => {
   // usernameが空か否かはisUniqueで確認するべきじゃないので、バリデート関数を別途用意すべき
@@ -148,15 +148,7 @@
     // 余計な事はせずにやること終わったらさっさとtrue or false返して終了する
-    if (results.length === 0) {
+    resolve(results.length === 0);
-      resolve(true);
-    } else {
-      resolve(false);
-    }
   });

更に修正

2018/12/14 09:44

投稿

miyabi-sun

スコア21158

test CHANGED Viewed

@@ -122,25 +122,7 @@
 const isUniqueUsername = (username) => new Promise((resolve, reject) => {
-  // 長いif文によるネストは可読性を下げるので、ガード節ですぐ返す
+  // usernameが空か否かはisUniqueで確認するべきじゃないので、バリデート関数を別途用意すべき
-  if (!username) {
-    // resolve(false)かreject(エラー理由)かは要検討
-    resolve(false);
-    return;
-  }
-  if (!password) {
-    resolve(false);
-    return;
-  }
@@ -152,7 +134,7 @@
   }, (error, results, fields) => {
-    // 余計な事はせずにやること終わったらさっさとtrue or false返して終了する
+    // errorがnull以外の時は、MySQLのコネクション失敗やらの申告なエラーなのでrejectで対応
     if (error) {
@@ -161,6 +143,10 @@
       return;
     }
+    // 余計な事はせずにやること終わったらさっさとtrue or false返して終了する
     if (results.length === 0) {

回答のコードがちょっと変だったので修正

2018/12/14 09:42

投稿

miyabi-sun

スコア21158

test CHANGED Viewed

@@ -120,7 +120,7 @@
 // 関数は外のbodyみたいな値に依存するのではなく、引数で受け付けたほうがいい
-const isUsernameUnique = ({username, password}) => new Promise((resolve, reject) => {
+const isUniqueUsername = (username) => new Promise((resolve, reject) => {
   // 長いif文によるネストは可読性を下げるので、ガード節ですぐ返す
@@ -148,7 +148,7 @@
     sql: "SELECT * FROM users WHERE username = ?;",
-    values: [body.username]
+    values: [username]
   }, (error, results, fields) => {
@@ -182,9 +182,9 @@
   // bodyをどっから取ってくるかは知らんけどこんな感じ
-  var isValidated = await isUsernameUnique(body);
+  var isUnique = await isUniqueUsername(body.username);
-  if (isValidated) {
+  if (isUnique) {
     console.log('ユニークなIDです');

＞＜

2018/12/14 09:39

投稿

miyabi-sun

スコア21158

test CHANGED Viewed

@@ -83,6 +83,10 @@
 実際にこれを走らせるとthenの意味はまるでなくて、その場で全てのPromiseが同時に走るわ、引き継がせるデータがちゃんとリレー出来てないわという有様で、
 意図したように動かないんじゃないの？という印象ですね。
+特に設計思想としてはpromise走らせた結果として`resolve(isValided)`を実行しながら返すという設計にすべきで、予め`let isValided = true;`を実行しておいて、Promiseの中身で外のスコープの変数を書き換えてfalseにするのはお前Promiseの意味ないじゃんということで落第です。