-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update message에 message_type 필드 추가 #12
Conversation
- host 상수로 선언 - 불필요한 svg 정리
|
||
async def disconnect(self, connection: UserConnection): | ||
self.manager.disconnect(connection) | ||
self.users.remove(connection) # 사용자 제거 | ||
if not self.users: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 조건문 별로라고 생각했는데 수정해주셨네요
감사합니다
from pydantic import BaseModel | ||
|
||
|
||
class MessageType(Enum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
열거형을 사용하는 경우를 잘 못봤는데 텍스트로 사용하는 경우 대신에 쓰는거군요
하나 궁금한게, http request할때 각 header에 persistent connection, set cookies할때도 보통 단순 텍스트가 아니라 열거형을 쓰나요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음.. 잘 아시겠지만 보통 문자열을 그냥 사용하면 실수할 가능성이 많아서 상수로 사용할 수 있을만한 도구를 쓰는 편이에요! 도구는 언어마다 달라서 적절한 걸 사용하면 되고 여기서는 Enum을 선택했다고 보면 됩니다 ㅎㅎ
어떤 도구를 사용하던 SSOT(Single Source Of Truth)를 두는 방향으로 선택하면 좋은 것 같아용
하나 궁금한게, http request할때 각 header에 persistent connection, set cookies할때도 보통 단순 텍스트가 아니라 열거형을 쓰나요
이런 경우도 보통 다 선언해서 사용해요! Spring 같은 경우는 HttpHeader에 Enum이 모두 정의되어있던 걸로 기억합니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문자열로 사용하면 데이터 변경이 이루어지기 쉬우니까 ENUM을 사용하자는 것은 이해했습니다.
방금 찾아보니까 단일 진실 공급원에 의해서만 데이터가 수정, 변경이 이루어져야 하는것이 SSOT라고 하는데,
문자열이 변경이 쉬워서 열거형을 사용하는것과 하나의 공급원로 데이터가 전달되어야 한다는 점이 무슨 관계인지 설명해 주실 수 있을까요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
환경 변수 개념으로 하나의 포인터를 바라본다 라고 보면 좋을 것 같은데 사실 아래처럼 문자열을 선언해두고 상수로 사용해도 돼요!
MESSAGE_TYPE_USER_MESSAGE = "USER_MESSAGE"
MESSAGE_TYPE_SYSTEM_MESSAGE = "SYSTEM_MESSAGE"
다만 이렇게하면 코드가 물리적으로 붙어있는 게 강제되진 않고 Enum을 사용하면 물리적으로 붙어있는 게 강제되고 reference로 코드를 찾아가기 쉬워져서 코드를 관리하기 깔끔해지는 것 같아용
python은 잘 모르겠지만 다른 언어에서는 Enum의 필드는 unique함이 보장되는 등 부가적인 기능도 있구요!
여기서 포인트는 아래처럼 static하게 값을 사용하는 경우를 방지하고 모두 하나의 포인터를 바라보게 하기 위함이라고 보면 됩니당
Message(message_type="USER_MESSAGE")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 하나 또 배워 갑니다 자세한 설명 감사해요
|
||
ws.onmessage = function (event) { | ||
const message = JSON.parse(event.data); | ||
if (message.username === "Root") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 위에서 username을 삭제 했는데 작동 안되는거 아닌가요..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 맞습니다 주석을 안 달아뒀네요 ㅠㅠ 체크 감사합니다! 👍
기존 로직에서 room_name을 넘기는 용도였던 것 같아서 이제 사용하지 않는 로직이라 그냥 두었는데 개선 작업에서 챙겨질 예정이에요!
화면상에서 전체 로직은 잘 동작하는 걸로 확인했습니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자바스크립트라서 살았네요 ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ
Summary
@falconlee236 새벽 작업이라 리뷰 시간이 안 맞을 것 같아서 미리 merge해둘게요! 코드 보고 후 리뷰 부탁합니다!