第一次程式碼審查

昨天，我的程式碼第一次被其他人審查。作為我正在參加的開源開發課程的作業之一，我們必須審查彼此的程式碼。在這個練習中，我與 Vinh 搭檔，他是我的一個朋友，碰巧也是一位非常優秀的程式設計師。我們的任務是測試和歸檔彼此在課程中一直在使用的命令列工具工作的問題。

Vinh Nhan關注

/維尼揚

非同步與同步方法

我們透過文字同步和透過 GitHub 問題非同步進行程式碼審查。我發現同步方法可以更快地得到結果，因為我可以諮詢程式碼作者，了解他們在編寫程式碼時為什麼採用某種方法，並立即得到答案。然而，非同步方法消除了在兩個人的日程安排中找到固定時間來完成工作的需要。

測試 Vinh 的程序

Vinh 創建了一個名為「barrierless」的命令列工具，它使用人工智慧將文字短語翻譯成其他語言，我認為這是一個很酷的主意。當我開始測試 Vinh 的程式時，它還處於早期開發階段，所以還沒有 README（現在有了，去看看吧！）。

git clone git@github.com:vinhyan/barrierless.git

cd barrierless

npm install

A feature I really liked is the colorful output text which makes the user experience a little bit more pleasant - something I neglected in my own program in trying to model it after CLI tools like git.

I read the package.json file to find out how the program should be run, and when it immediately crashed I realized I forgot to add the API key as an environment variable. After adding my API key, the program ran without errors, although I did find an interesting quirk - the program defaults the output language to English, so if you didn't specify one, and the input was in English, it seemed to choose a language to translate to on its own - either randomly, or based on context from the input.

I opened a few other issues, mostly to do with improving code quality:

• A missing try/catch block around an async function call

Uncaught exception in index.js #7

uday-ranaposted on Sep 12, 2024

index.js contains the following async function calls which are not wrapped in a try/catch block and may lead to an uncaught exception:

export async function main(text, targetLang) { const chatCompletion = await getGroqChatCompletion(text, targetLang); console.log(chatCompletion.choices[0]?.message?.content || ''); } ... program ... .action(async (text, options) => { console.log(chalk.blue(`Translating ${text}...`)); await main(text, options.language); });

登入後複製

View on GitHub

• Some suggestions to make code easier to understand

Could simplify code #8

uday-ranaposted on Sep 12, 2024

Some changes may be made to to the project make it easier to understand and work on:

• [x] Move Groq configuration above program initialization with commander
• [x] main() seems unnecessary since it contains two lines of code and there are more lines of code involved in creating and invoking the function than if it was omitted
• [ ] prompt.js seems unnecessary since it just contains a single function which places arguments into a template literal and returns them
• [x] Exporting main() and getGroqChatCompletion() seems unnecessary

View on GitHub

• Adding a comment to explain the use of both import and require statements

Add comments explaining mixed import/require #9

uday-ranaposted on Sep 12, 2024

이 프로젝트는 가져오기를 사용해야 하는 초크 모듈과 package.json에 가져오기를 사용하면 오류가 발생하기 때문에 ES6 가져오기와 CommonJS 요구 사항을 모두 사용합니다. 이를 설명하는 댓글을 추가해 주시면 도움이 될 것입니다.

GitHub에서 보기

내 차례

다음은 제가 리뷰할 차례였습니다. 어떤 문제가 나타날지 확신할 수 없었지만 Vinh은 결국 제가 주의를 기울이지 않았던 많은 문제를 발견하게 되었습니다.

• README의 지침에서 도구 앞에 노드를 붙일 필요가 없도록 또 다른 옵션으로 npm 링크를 추가합니다.

README.md에는 `npm link` 실행 지침이 포함되어 있지 않습니다. #2

빈얀게시일: 2024년 9월 12일

README.md 파일에는 CLI 도구의 로컬 개발 및 테스트에 필요한 npm 링크 실행 지침이 없습니다

GitHub에서 보기

• commander.js를 사용한 불필요한 명령 할당

CLI에는 하위 명령이 없으므로 `program.command("run")`은 필요하지 않습니다. #3

빈얀게시일: 2024년 9월 12일

GitHub에서 보기

• 변수명 오타

변수 이름 오타 #4

빈얀게시일: 2024년 9월 12일

index.js 31행: 변수 이름에 오타가 있습니다: responseStream

GitHub에서 보기

결론

아주 잘했다고 생각했지만 항상 놓쳤을 수 있는 버그나 개선할 수 있는 기능이 있다는 것을 보여주려고 합니다. 내가 작성한 코드를 새로운 눈으로 자세히 살펴볼 수 있어서 정말 좋았습니다. 지금은 오타를 수정하고 README를 업데이트했지만 다른 문제는 테스트가 필요하므로 버전 0.1을 출시하기 전에 해당 문제를 해결할 계획입니다.

以上是第一次程式碼審查的詳細內容。更多資訊請關注PHP中文網其他相關文章！