Skip to content
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

[Week1] 멋사 레스토랑 #2

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[Week1] 멋사 레스토랑 #2

wants to merge 5 commits into from

Conversation

sanghyunna
Copy link
Member

@sanghyunna sanghyunna commented Sep 16, 2023

Node Conference - Week1 HW

설계:

  • 요리 하나하나를 객체로 인식하는 것이 편리할 것이라 생각해 객체지향으로 설계했습니다.
  • 요리사와 홀알바가 한 명씩이고, 하나의 작업만을 처리할 수 있으므로 각각의 큐를 주었습니다.
  • 각 객체의 요리/서빙이 종료되면 EventEmitter를 호출해 각 큐를 진행 시킵니다.
  • [추가] 가독성을 위해 파일과 클래스를 모두 분리했습니다.
  • [추가] 동일 EventEmitter() 인스턴스가 호출한 이벤트만 listen할 수 있어, 공통 인스턴스를 선언한 후 export 했습니다.
  • [추가] 요리사와 홀알바를 별도 클래스로 분리했습니다. 요리사/홀알바의 큐는 요리사/홀알바가 직접 관리합니다.
  • [추가] 실제로 요리사와 홀알바가 여러 명인 상황을 구현하는 게 과제의 취지는 아닌 것 같아, chooseChef()chooseServer()에 로드 밸런싱을 위임했고, 각 1명의 요리사/홀알바를 데리고 있는 것으로 가정해 구현했습니다.
  • [추가] 왜인지는 모르겠지만 emitter 선언부를 수정한 커밋이 PR에 안뜨네요.. const 선언으로 수정했습니다..!

결과:

[개선]
image

@sanghyunna sanghyunna requested a review from d0422 September 16, 2023 17:44
@sanghyunna sanghyunna self-assigned this Sep 16, 2023
Copy link
Contributor

@d0422 d0422 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사장님~ 제가 고수 빼달라고 했잖아요~
너무 잘하셨습니다!
다만, 조금 더 개선할 여지가 있는 부분을 드리자면 입력과 출력, 데이터 처리부분이 조금 결합되어있는 모습을 확인 할 수 있었는데요, 이부분을 좀 더 분리해보는건 어떨까 싶습니다.

이 부분이 붙어있게되면, 입출력 명세가 바뀐다던지 하는경우가 생길때 수정할 부분들이 많아지거든요!

그리고 파일로 분리해보는건 어떨까요?

Comment on lines 11 to 17
const Status={
WAITING: 0,
COOKING: 1,
COOKED: 2,
SERVING: 3,
SERVED: 4,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 Status가 변경될 수 있는 위험을 줄이기 위해 얼리기를 추천드립니다!

const STATUS=Object.freeze({
  WAITING: 0,
  COOKING: 1,
  COOKED: 2,
  SERVING: 3,
  SERVED: 4,
});

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

Comment on lines 67 to 71
this.chefQueue = [];
this.serverQueue = [];
this.finishedQueue = [];
this.isCooking = false;
this.isServing = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금도 너무 좋지만, 조금 개선할 여지가 있는부분을 말씀드리자면
레스토랑이 셰프 큐와 서버큐, 완료 큐, 그리고 요리 상태와 서빙 상태를 갖고 있네요. 레스토랑이 모든 정보를 다 갖게 되는데 이로인해서 이후에 유지보수, 기능추가에 문제가 생길 수도 있을 것 같습니다.

예를들어서 셰프가 한명 더 생긴다던지, 서버가 한명 더생기는 경우이죠.
이런경우를 고려해서 역할을 조금 더 분리 해보는 건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트대로 추가 구현했습니다! 확인 부탁드려요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants