[TOC] ## Code review #1 by 季芳 ### q6 * 目前把問題寫死，所以問句都是"Please enter a name in English: "，使用者會不知道已經問到第幾個名字了 * 重複的動作可以包成小function * 且因為function有名字，也能讓程式碼更好讀 * for迴圈幾乎都可以用Array methods解決 * map, filter, reduce, from * reduce可以看阿傑的鐵人文章 * 有些人也會把各式各樣的驗證再包成一個function * 因為如果小驗證很多的時候就會太長 * 跟型別轉換有關的變數宣告可以少部份使用匈牙利命名 ### q8 * 忘記要用遞迴寫了!! * 可以試試看不同的切入點：轉字串後處理，或是直接用數字處理 ### q9 * 季芳也曾經被問如果陣列長度不一樣的話要怎麼處理，但可能可以不用涵蓋到這件事，畢竟這算是增加需求了 ### q10 * 找質數反證法 > 一個自然數若恰有兩個正因數（1及此數本身），則稱之為質數。 > 找質數反證法：找到該值的開根號並取餘數等於0就不是質數。 > 0,1不是質數，2是所有質數中唯一偶數。 * 如果for迴圈不好讀，但又不知道要怎麼重構，可以先拆成function ## Code review #2 by Jade ### q8 覺得沒什麼問題，命名命得很清楚 不過 test 你好像沒有放新命名的 function 所以會有錯XD ### q10 1. isPrime 沒有判斷如果是 1 的話 return false 或許是這樣你才在 Array.from element + 2 嗎？ ```javascript const getNumbers = Array.from( Array(maxNumber - 1), (element, index) => index + 2 // 從2開始放進array ); ``` ```javascript // isPrime後來改成加入 if (number < 2) { throw new Error("請輸入大於1的整數"); } ``` 2. toSorted() 有點太新了，如果要用的話，專案要 pin node@20 這樣 install 才可以使用 - volta pin node@20 - [volta介紹](https://ithelp.ithome.com.tw/articles/10306738) - [volta加入node版本](https://docs.volta.sh/guide/understanding#managing-your-project) - 其實每個專案要用的版本應該都要放在 package.json 上，這樣要使用的人 install 的時候才不會有問題 - [直接改package.json](https://docs.npmjs.com/cli/v10/configuring-npm/package-json#engines) 4. 覺得你的程式已經很簡潔了，不過像是要拿 1~50 的數字把他裝起來一個變數會更語意化 ```javascript // 原本全部串在一起 const primeNumbers = Array.from( Array(maxNumber - 1), (element, index) => index + 2 ).filter(isPrime); // 改成 const getNumbers = Array.from( Array(maxNumber - 1), (element, index) => index + 2 ); const primeNumbers = getNumbers.filter(isPrime); ``` ## Code review #final by Chris ### q6 * function命名綁架參數 * 現在是流程設定和資料一起丟進去 * 流程控制跟資料輸入要分開，先記得這件事，現在可能還比較沒辦法體會 * 我目前的寫法看起來像pipe(流程)，但第一個參數卻不是function，不一致 * getAnswersAndPrintResults的三個參數，很像在設定它 * getAnswersAndPrintResults應為副程式，跟main相關 * 因為我也沒有要測試他，所以不用分成別的模組 * 邏輯界面分離就好，不要為了包模組而分檔 ```javascript function main() { // 原本寫的 const questions = [ "Please enter the 1st name in English: ", "Please enter the 2nd name in English: ", "Please enter the 3rd name in English: ", "Please enter the 4th name in English: ", ]; getAnswersAndPrintResults( questions, // 丟了四個字串，像是丟了資料的感覺 validationsInQ6, oddLettersOfFirstAndThirdNames ); // 改成 // 用參數去命名(請見下方說明) askQuestionsAndDoSomething( { question: questions, validation: validationsInQ6 }, oddLettersOfFirstAndThirdNames ); } main(); ``` * 命名議題：什麼時候用參數命名、什麼時候用函式回傳的結果命名 - 當回傳值指派給某個變數時，函式可以命名為回傳的結果 - 否則這個函式就是一個「動作」，利用參數去做一些動作，所以應該要依照參數去命名 ```javascript // 將sin(90)這個「sin值」指派給a，所以sin這個函數命名為sin var a = sin(90); // 因此我的function如果有回傳值並指派給某個變數，才適合命名為getAnswersAndPrintResults const answer = getAnswersAndPrintResults(...) ``` #### getAnswersAndPrintResults * indexOfQuestion要當作getAnswers的參數傳入 * 寫法要pure，意思是函式不要影響到全域變數 * 寫法pure還有一個優點是把整塊function搬走會比較簡單 ```javascript // 原本寫法 export function getAnswersAndPrintResults(questions, validations, doSomething) { // ... let indexOfQuestion = 0; function getAnswers() { if (indexOfQuestion === questions.length) { // ... } else { rl.question(questions[indexOfQuestion], (answer) => { try { // ... indexOfQuestion++; getAnswers(); } catch (error) { // ... getAnswers(); } }); } } getAnswers(); } // 可以改成 export function getAnswersAndPrintResults(questions, validations, doSomething) { // ... function getAnswers(indexOfQuestion) { if (indexOfQuestion === questions.length) { // ... } else { rl.question(questions[indexOfQuestion], (answer) => { try { // ... getAnswers(indexOfQuestion + 1); } catch (error) { // ... getAnswers(indexOfQuestion); } }); } } getAnswers(0); } ``` ### 驗證 #### isEmpty * 有三種作法 1. 我的寫法，直接看input是不是為空字串 2. 判斷typeof是否為string, 是的話長度是否等於0 3. 直接判斷長度是否等於0，若沒有長度的話會是undefined * 驗證不能只考慮readline輸入為字串的情況 * 有關驗證的問題，總而言之就是，因為是「這個型別」，所以要對「這個型別」的輸入做驗證，所以驗證基本上就是跟型別綁在一起 * 驗證會依照它的名字去假定傳進來的是字串還是數字，例如isIntegerNumber就假定傳進來的是數字，所以轉型別的事情不是給驗證處理，而是放在驗證外 * 養成注意型別的習慣 * 拋出的錯誤，其型別應固定為Error object ```javascript export function isEmpty(input) { if (input === "") { throw new Error("請輸入內容"); } } ``` #### isEnglish * 魔法數字：64是什麼？91是什麼？要宣告成65=A, 90=Z ```javascript export function isEnglish(input) { for (let i = 0; i < input.length; i++) { // 舊的 if (input.charCodeAt(i) > 64 && input.charCodeAt(i) < 91) // 改成 const A = 65; const Z = 90; const a = 97; const z = 122; const space = 32; if (A <= input.charCodeAt(i) && input.charCodeAt(i) <= Z) { continue; } else if (a <= input.charCodeAt(i) && input.charCodeAt(i) <= z) { continue; } else if (input.charCodeAt(i) === space) { continue; } else { throw new Error("Please enter something in English"); } } } ``` #### isSpaceInFirstLast * 可以改用`"".trim`處理完再判斷長度是否跟原本一樣 * `"".trim`等同於String.prototype.trim()，用""代表String更為簡潔 * 注意不能改到原本的string ```javascript // 原本的寫法 export function isSpaceInFirstLast(input) { if (input.at(0) === " " || input.at(-1) === " ") { throw new Error("Please do not enter space in the first and last words"); } } // 可以改為 export function isSpaceInFirstLast(input) { const trimmedInput = input.trim(); if (trimmedInput.length !== input.length) { throw new Error("Please do not enter space in the first and last words"); } } ``` ### 其他問題 1. 直接return 用array method處理的陣列？還是先宣告一個變數，再return這個變數 - 要看這些method直不直覺 - 排版上要讓一行code只做一件事 - code讀起來像英文一樣就是好code - 要去習慣把code縮小，而不是多到超出視野，這也是為什麼Chris一開始不推薦大家使用雙螢幕 ```javascript // 原本寫的 function oddLettersOfName(name) { const oddLetters = Array.from(name) .filter((element, index) => index % 2 === 0); // 這個地方就比較適合用宣告變數，讓人家一看就知道這是要拿到奇數的字母 return oddLetters.join(", "); // 可以改成 // 直接return用array method處理的陣列 function oddLettersOfName(name) { // 把filter內的函式獨立出來，命名，賦予涵義 // 這個function擺在用到它的附近，在視線範圍內就可以 function oddLetters(element, index) { return index % 2 === 0; } // 一行code只做一件事 return Array.from(name) .filter(oddLetters) .join(", "); } ``` 2. 被說用到新的array method，node怎樣的版本算是新？ - 註記專案使用的版本就好，不用記哪些method算是新的 - 這是系統性問題，不是語法問題 3. array method沒用到element，我看小銘用_代替，是公認的作法嗎？ - 不是公認的作法，不過算是一種流派 - Chris會寫某個字母，例如o或x當做圈圈叉叉，代表一個記號 - 就像當code沒有很長的時候index可以用i代表一樣 4. for迴圈很少用到了嗎？ - 幾乎不寫了 - for迴圈是知道長度的時候使用的 - 為什麼會知道長度？因為是array - 因為是array，所以可以使用array method - 可以參考Chris的[鐵人文章](https://ithelp.ithome.com.tw/articles/10209105) 5. isPrime 小於2的數是錯誤還是false - 看數字小於2是否為錯誤流程 - 照理說只要輸入數字就應該可以跑，所以不該丟錯誤