second commit #1
Conversation
There was a problem hiding this comment.
안녕하세요! @wjdtnghks123
게임이 오류 없이 잘 작동 되어서 재미있게 플레이해볼 수 있었어요!
수고하셨습니다! 🥳
저 또한 파이썬 초보 사용자이기 때문에 고급 문법의 사용 등을 보기 보다는
협업을 위한 코딩 스타일을 맞추거나 프로젝트의 유지 보수가 용이 하도록
개선할 수 있는 방향으로 초점을 맞추어 리뷰를 진행했습니다.
제 의견 또한 틀릴 가능성이 높으며 정답도 아닙니다!
wjdtnghks123님의 좋은 의견이 있다면 코멘트 답장을 통해 언제든지 의견을 남겨 주세요!~
코딩 스타일
협업 과정에서 코딩 스타일을 맞추면 코드의 가독성도 개선할 수 있고,
다른 사람들이 내 코드를 쉽게 이해할 수 있도록 도와줄 수 있다고 생각합니다.
이번의 경우는 PEP8 스타일을 기준으로 리뷰를 진행했습니다.
코드 리뷰를 참조하실 때, PEP8 가이드 문서를 가볍게 훑어 보시면 더욱 도움이 될 수 있을 것입니다.
추후, 협업 시에는 기존에 프로젝트에서 사용되는 스타일을 참조하거나,
동료와 사용할 스타일을 협의하는 과정을 통해 맞춰 나가는 것도 좋은 방법이 될 수 있을 것으로 생각됩니다.
매번 가이드를 찾아가며 코딩 스타일을 맞추는 것은 쉬운 일이 아닐 수 있다고 생각합니다.
각종 lint프로그램을 사용하면 자동으로 스타일 룰을 벗어난 코드 조각을 찾고 수정할 수 있도록 도와줍니다.
제가 사용한 PyCharm IDE의 경우 PEP8을 기준으로 코딩 스타일을 검사 및 수정해주는 lint가 설정되어 있는데,
자주 사용하시는 IDE 나 코드편집기의 설정을 확인해 보시면 좋을 것 같습니다.
코드 설계
코드 리뷰 과정에서 언급하였지만, 현재 프로젝트 구조는 GUI 코드와 게임 로직이 동일한 클래스 내에 함께 구현되어,
게임 로직의 검증, CLI와 같은 다른 방식의 UI로 확장 개발이 어려운 구조라고 생각합니다.
이 문제는 wjdtnghks123 님 만의 문제가 아닙니다.
저를 포함한 다른 프로그래머들도 GUI 프로그래밍 과정에서 코드 설계 시 많은 고민을 하게 되는 부분이 될 수 있다고 생각합니다.
전통적인 해결 방법은 #Model-View-Controller (MVC 패턴)라고 불리는 코드 설계 방법이 존재합니다.
이 방법을 찾아보시면 문제 해결에 도움이 될 수 있을 것으로 생각합니다.
개발방법론 을 통해 좋은 코드 설계 구조로 수렴해 나가는 방법도 있습니다.
제가 소개해 드리는 방법은 테스트 주도 개발 방법론 (TDD)라고 부릅니다.
이 키워드를 구글에서 찾아보시면 단위 테스트를 활용한 TDD 방법이 많이 소개되어 있으니
시도해 보시면 도움될 수 있을 것으로 생각합니다!
의존성 관리
프로젝트를 구동하기 위해 어떤 모듈이 필요한지, 어떻게 설치하는 지에 관한 문서를 찾을 수 없었습니다.
README 등의 문서를 통해 필요한 의존성이나 실행 환경 등을 설명해 주시면
처음 사용자가 손쉽게 프로젝트를 구동하는데 큰 도움이 될 수 있으리라 생각 합니다.
(인간의 기억력이 좋지 않을 수 있으니 미래의 자신을 위한 문서가 될 수 있습니다)
파이썬 패키지 관리자를 통해 손쉽게 의존성을 설치할 수 있도록 환경을 구축해 주시면
더욱 편리해질 수 있을 것 같아요! 이와 관련하여 설명된 문서를 첨부해 드리니 개선 시 활용해 보세요!
pip.pypa.io - requirements files
pyqt.py
Outdated
| import sys | ||
| from PySide6.QtWidgets import (QLineEdit, QPushButton, QApplication, | ||
| QVBoxLayout, QDialog, QTextEdit ) | ||
| from PySide6.QtGui import QColor | ||
| import random | ||
|
|
There was a problem hiding this comment.
style guide 에서 import 의 순서에 대한 설명을 찾았습니다.
순서는 다음과 같습니다.
- Standard library imports (sys, random 모듈이 해당될 수 있어요)
- Related third party imports (PySide, PyQt 모듈이 해당될 수 있어요!)
- Local application/library specific imports
파이썬 모듈의 import 코드의 작성 순서를 위와 같이 개선 해보는 것은 어떨 까요?
There was a problem hiding this comment.
링크 걸어주신 문서 읽은 후 import문의 순서를 바꾸어 수정하였습니다!! 😁
pyqt.py
Outdated
| import sys | ||
| from PySide6.QtWidgets import (QLineEdit, QPushButton, QApplication, | ||
| QVBoxLayout, QDialog, QTextEdit ) | ||
| from PySide6.QtGui import QColor | ||
| import random | ||
|
|
There was a problem hiding this comment.
PEP328 - Rationale for Parentheses
2 ~ 3번 행의 코드 처럼 어느 모듈에 대한 다중 import 가 이뤄질 때, 참조할 수 있는 가이드 라인을 다음과 같이 찾았습니다.
- 백 슬래쉬(/) 를 활용한 방법
- 다중 import 문을 작성 하는 방법
- 현재 사용 하신 방법과 동일한 괄호를 이용한 그룹핑 방법
3번 행에서 사용하신 코드 들여쓰기 방법 케이스와 매칭되는 가이드를 찾지는 못했으나, 이와 유사한 코드 들여쓰기 가이드를 찾았습니다.
다른 방법의 코드 들여 쓰기도 한번 확인 해보면 좋을 것 같아요!
현재 사용 하신 방법을 선택(혹은 선호) 하는 이유는 무엇 인가요?
There was a problem hiding this comment.
링크 걸어주신 문서 읽은 후 수정하였습니다!! 😁
import 를 다중 import 문을 작성하는 방법으로 개선하였습니다.
만약 위 코드처럼 그룹핑을 사용한다면 들여쓰기를 사용하여 정렬해야한다는 것을 알게 되었습니다.
개선 방안 :
from PySide6.QtWidgets import (QLineEdit, QPushButton, QApplication,
.....................................................QVBoxLayout, QDialog, QTextEdit )
(공백을 넣으면 사라져서 이렇게 했습니다😥)
pyqt.py
Outdated
| from PySide6.QtGui import QColor | ||
| import random | ||
|
|
||
| class Form(QDialog): |
There was a problem hiding this comment.
코드를 작성할 때 빈 줄을 넣는 방식 등을 사용 하여 코드 레이 아웃을 잡는 가이드 라인을 찾았 어요!
Surround top-level function and class definitions with two blank lines.
클래스의 정의 부를 모듈 import 부분과 메인 함수 영역과 구분 지어 보면 어떨까요?
There was a problem hiding this comment.
링크 걸어주신 문서 읽은 후 수정하였습니다!! 😁
클래스를 정의할 때 상위 함수(여기서는 import 부분) 사이에 2줄
클래스 내 함수를 정의할 때 1줄을 비워야 한다고 저는 이해했습니다.
혹시 틀렸다면 comment 부탁드립니다!!
pyqt.py
Outdated
|
|
||
| class Form(QDialog): | ||
| def __init__(self, parent=None): | ||
|
|
There was a problem hiding this comment.
class 문을 작성하고 줄 바꾼 후 tab 을 눌러 def 문을 작성하였습니다.
class 문과 def 문 사이에 blank line을 추가했으면 더 좋았을 것 같습니다.😅
수정하였습니다!
pyqt.py
Outdated
|
|
||
| # layout 구성요소 | ||
| super(Form, self).__init__(parent) | ||
| self.user_edit = QLineEdit("당신의 선택, form : 마지막 띄어쓰기 한 칸 , ex : '가위 ' ") |
There was a problem hiding this comment.
사용자가 기본 메시지를 지우지 않고도 "가위 ", "바위 ", "보 " 를 입력할 수 있다면 사용자 경험이 더욱 개선될 수 있을 것 같아요!
#Placeholder
There was a problem hiding this comment.
Placeholder 에 대해 알아본 후 수정하였습니다!! 😄
저도 다른 프로그램을 사용할 때 이러한 경험을 하였는데 막상 만들 때는 생각하지 못 했습니다.
기존 문자를 지우고 작성해야하는 불편함을 없앨 수 있게 되었습니다.
좋은 정보 감사합니다! 👍
pyqt.py
Outdated
| self.who_winloss() | ||
|
|
||
| # 사용자 입력, 컴퓨터 입력 비교 후 결과 도출 | ||
| def who_winloss(self) : |
There was a problem hiding this comment.
PEP8 - Prescriptive: Naming Conventions
겸사 겸사 파이썬에서 사용되는 네이밍 컨벤션을 한번 찾아봅시다!
컴퓨터 프로그램에서 변수, 함수, 클래스 등의 네이밍은 어떤 의미로 사용될까요?
There was a problem hiding this comment.
링크 걸어주신 문서 읽은 후 수정하였습니다!! 😁
원래 생각은 def who_winlose()로 할 생각이었습니다.
최초 작성할 때 loss로 작성 하였고 이후 자동완성 기능을 사용하여 발견하지 못하였습니다. 😥
함수, 변수 이름은 짧고, 단어를 _(밑줄)로 구분해야 한다는 사실을 발견했습니다.
pyqt.py
Outdated
|
|
||
| # 그외 승부가 나는 경우 | ||
| else : | ||
| if str(self.user_edit.text()) == "가위 " : |
There was a problem hiding this comment.
현재와 같은 구조 에서는 만약 가위 바위 보를 Rock, Paper, Scissors 로 바꿔야 할 때, 코드에 있는 가위, 바위, 보 를 하나씩 하나씩 찾아서 수정 해야 할 수 있는 문제가 발생될 수 있다고 생각 합니다.
There was a problem hiding this comment.
만약 만든 프로그램이 계속 업데이트를 진행할 예정이라고 가정한다면 말씀 하신 것처럼 문제가 발생할 것입니다.
코드 작성 당시에는 생각하지 못하였는데, 정택님의 피드백 덕분에 알게 되었습니다🌟
아래와 같이 수정하였습니다.
가위, 바위, 보를 묶어 list 로 관리하면 좋을 것 같습니다.
- 가위, 바위, 보의 이름을 변경할 경우( ex. 영어, 중국어, 일본어 등) - list 를 수정한다.
pyqt.py
Outdated
| def print_image(self) : | ||
| # 결과 경우의 수 | ||
| result_text = [" win " , " Lose" , " Draw"] | ||
| self.image.setFontItalic(True) | ||
| self.image.setFontPointSize(180) | ||
| if self.result_index == 0 : | ||
| self.image.setTextColor( QColor(0,0,255)) | ||
| elif self.result_index == 1 : | ||
| self.image.setTextColor( QColor(255,0,0)) | ||
| elif self.result_index == 2 : | ||
| self.image.setTextColor( QColor(0,255,0)) | ||
| else : | ||
| # 입력 form이 맞지 않는 경우 | ||
| self.image.setTextColor( QColor(255,255,255)) | ||
| self.image.setFontPointSize(30) | ||
| self.image.setText("please fill in the form ") | ||
| return | ||
| self.image.setText(result_text[self.result_index]) | ||
| return | ||
|
|
There was a problem hiding this comment.
PEP8 - Whitespace in Expressions and Statements
위 코드 중 함수의 정의부 ":" , 배열의 element 간 ",", QColor(255,255,255) 내부 에서 사용한 공백의 사용 기준을 설명해주세요!
There was a problem hiding this comment.
링크 걸어주신 문서 읽은 후 수정하였습니다!! 😁
함수 정의부와 QColor내부 공백은 문서를 읽고 수정 하였고, 이외 같은 잘못을 한 부분도 수정하였습니다.👏
result_text = [" win " , " Lose" , " Draw"] 이 경우는 글자 크기와 출력창의 크기를 고려하여
글자마다 띄어쓰기를 다르게 하였습니다.
pyqt.py
Outdated
|
|
||
| # 그외 승부가 나는 경우 | ||
| else : | ||
| if str(self.user_edit.text()) == "가위 " : |
There was a problem hiding this comment.
게임 로직이 잘 동작하는지 코드를 검증 해볼 수 있는 방법이 있을 까요?
| QVBoxLayout, QDialog, QTextEdit ) | ||
| from PySide6.QtGui import QColor | ||
| import random | ||
|
|
There was a problem hiding this comment.
파이썬 패키지 의존성을 손쉽게 관리할 수 있는 방법은 무엇이 있을 까요?
|
수고하셨습니다. 단 시간에 많은 부분을 개선해주신 것이 보였습니다. 🤩🥳🤩🥳 |
안녕하세요 @smart8612 님
사제동행 강좌를 통해 만나게 되어 반갑습니다.
사용자 입력을 받아 컴퓨터와 하는 가위바위보 게임을 만들었습니다.
모든 코드를 파일 하나에 담아두었습니다. 코드를 이해하실 수 있도록 주석을 달았습니다.
PySide6==6.4.0.1
PyQt6==6.4.0
python==3.10.6
제가 누군가가 보기 위한 코드를 작성한다는 게 처음이라 어떻게 할 지 몰라서
제가 아는 선에서 노력을 해보았는데 많이 부족하다고 느껴집니다.
많이 부족하고 엉망이라 고칠 부분이 많이보입니다.
대략적으로 말씀해주시면 제가 열심히 고쳐보겠습니다.