---
# System prepended metadata

title: C_Q6-Q10 code review
tags: [CodeReview]

---

[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是否為錯誤流程
    - 照理說只要輸入數字就應該可以跑，所以不該丟錯誤