Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e65d90d22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = n1 / n2 | ||
| return round(result, 10) | ||
|
|
||
| except ValueError: |
There was a problem hiding this comment.
The input-validation path only catches ValueError, but float(...) also raises TypeError for common non-numeric values like None. In that case the function throws instead of returning the documented user-facing error message, so callers can crash even though this change claims to handle invalid inputs. Expanding this handler (or pre-validating inputs) is needed to make invalid-input handling reliable.
Useful? React with 👍 / 👎.
LEEMINGU03
requested review from
Donghwan814,
dohun0310,
leesj0706 and
seojoon1
and removed request for
1028ragon,
127lucas,
haonkr and
ohsanghy09
haonkr
force-pushed
the
team-1
branch
2 times, most recently
from
aede537 to
13207e5
Compare
|
안녕하세요 민구님! 이번에 코드 리뷰를 담당하게 된 김동환이라고 합니다 :) 이번 리뷰에서는 민구님이 작성해주신 코드에 대한 리뷰를 포함해 comment 달아주신 부분은 잘 지켜주셨는지 등 Git, GitHub를 어떻게 활용하고 어떻게 예외 처리 방식을 구현했는지에 대해 중점을 두고 리뷰를 진행하도록 하겠습니다. 이번 과제를 통해 Git, Github 사용법에 익숙해지면서 협업이 무엇인지 조금이나마 알아갈 수 있던 시간이 되었으면 좋겠습니다! |
| if n2 == 0: | ||
| return " 0으로 나눌 수 없습니다." | ||
|
|
||
| # 3. 계산 및 소수점 오차 보정 (10자리) |
There was a problem hiding this comment.
먼저, 이 부분에 대해서 민구님에게 질문 드리고 싶은 부분이 있습니다.
지금 주석 달아주신 부분인 "계산 및 소수점 오차 보정 (10자리)" 이 의미가 제가 이해한 내용으로는 나눗셈 결과를
소수점 10자리까지 반올림해서 반환해주는 의미로 저는 이해를 했습니다.
우선 제가 이해한 부분을 바탕으로 질문을 드리자면 10자리까지 남기고 반올림을 하시는 의도가 무엇인지 궁금합니다. 우선 float을 통해서 숫자를 받고 나누는 과정에서 소수점이 10자리가 넘어가는 경우가 당연히 존재할 것이라고 생각합니다. 현재 다른 PR 올린 내용에서 곱셈을 예시로 들면 소수점 3번째 자리까지 받게끔 코드가 작성되어 있는데 10자리까지 결과가 나오면 나눗셈만 왜 10자리까지 나오지? 하고 의문이 들 것 같다고 예상합니다. 혹시 이 의도가 있는지 궁금합니다.
There was a problem hiding this comment.
float의 노이즈 방지로 10자리로 잡았었습니다.
-> 8자리 or 9자리등 다른 것도 가능한데 10자리로 잡은 이유는 어림잡아 잡았던거 같습니다.
소수점 몇번쨰까지 통일하자라는 것은 팀원 분들과 소통하지 못한것 같습니다
사용자 입장에서 소숫점 자리가 다르면 시각적으로 불편 뿐만 아니라 의문을 가질 거라는 것 이해 할 수 있었습니다 . (여기 까지 생각하지 못하였습니다.)
There was a problem hiding this comment.
협업에서 가장 중요한 점은 팀원들과의 소통을 통해서 합의된 결과를 도출해가는 과정이 중요하다고 생각합니다. 지금 코드가 문제가 되는 코드는 아니지만, 이러한 부분에서 팀원과의 소통을 통해서 합의된 결과를 도출해서 정하는 과정을 배우셨으면 좋을 것 같아서 질문 드렸던 내용이었습니다!
| result = n1 / n2 | ||
| return round(result, 10) | ||
|
|
||
| except ValueError: |
There was a problem hiding this comment.
이 부분에 대해서 질문드리고 싶은게 있습니다.
우선 Chat GPT PR 리뷰를 사용해주신 접근은 매우 좋다고 생각이 듭니다. AI 리뷰를 사용하신 이유와 리뷰 달아주신 내용을 보고 어떤 부분에 대한 리뷰를 달아줬는지 확인 하시고 이해를 하셨을지 궁금합니다.
요약하자면 지금 코드는 ValueError만 잡고 있고 float()은 TypeError도 발생하고 있다고 TypeError도
같이 처리해야 한다는 내용입니다. 이 부분에 대해서 TypeError, ValueError 처리 하는 것이 무엇이고 어떤 차이를 가지고 있는지도 학습 해보시면 좋을 것 같습니다.
There was a problem hiding this comment.
ValueError는 타입은 맞지만 값이 잘못됐을떄
예를 들어 float("3.3")은 가능하지만 float("abc") 처럼 숫자로변환이 안될떄 발생한다고 알고 있습니다.
TypeError는 타입이 맞지 않을떄 발생 한다고 알고있습니다.
(현재 코드에서 list, None등)
입력이 항상 문자열이 올거라고 가정하고 작성하여
ValueError만 생각하고 TypeError를 놓치게 된거 같습니다.
There was a problem hiding this comment.
지금은 충분히 단순 계산기 기능 구현이기에 ValueError 부분만 고려하셨을 것이라 생각이 듭니다. 그러나 추후에 규모 있는 프로젝트 진행 시에는 TypeError에 대한 부분도 같이 생각해보면서 코드를 구현하실 수 있는 좋은 경험이 되셨으면 좋겠습니다! 😊
|
다른 분들도 공통적으로 했던 실수인 것 같은데, PR을 처음에 main 브랜치로 올리셨다가 이후에 team-1 브랜치로 변경하신 것으로 보입니다. PR을 올릴 때 team-1 브랜치를 사용하는 이유와, 이러한 방식을 사용하는 것이 왜 좋은지에 대해 한 번 더 찾아보시길 바랍니다. 또한, 협업 과정에서 이 방식이 왜 필요한지 충분히 이해하시고, 이후 프로젝트 진행 시에도 잘 활용해주셨으면 좋겠습니다. |
|
이 부분에서 assigned에 한 분을 설정해주시고 그 다음에 나머지 분들을 추가하셨는데 혹시 Assignees 의미가 무엇인지 그리고 추가적으로 Reviewers등 무슨 차이고 각각 어떤 역할을 하는지에 대해서도 한 번 더 찾아보시면 좋을 것 같습니다. |
|
작성해주신거에서 관련 issue를 설정해 주신것은 매우 좋은 의도라고 생각합니다. 우선 주요 작업 내용에서 어떤 작업 내용을 다루셨는지에 대해서 list로 작성해주신 것은 좋습니다. 한가지 더 나아가 완료되는 조건이나 작업을 내가 잘 수행했는지 셀프 체크 할 수 있도록 Task list도 사용해보시면 좋을 것 같습니다. |
이 프로젝트를 하면서 깃뿐만아니라 당연시 생각했던 것들과 개념을 알기만 하고 왜 사용하는 지에 대해 깊게 생각하지 않았던 것 같습니다. |
제가 드린 질문들에 답을 바라고자 드린 것은 아닙니다. 민구님께서 Git, Github에 대해 기본적인 충분한 지식과 활용 능력을 가지고 있다고 판단이 됩니다. 제가 드린 질문이 민구님께서 제가 드린 질문에 대해서 이것을 왜 질문하셨을까? 하면서 기능만 사용해 보는 것이 아닌, 충분한 이해를 바탕으로 더 잘 활용해가시길 기대하며 드린 질문들이었습니다. 과제 제출하시느라 굉장히 수고 많으셨습니다! 😊 |
6 participants
주요 작업 내용
관련 이슈
closes #6