Skip to content

Conversation

@lamodadite
Copy link

테스트 코드 통과하는것 확인 했습니다! 확인 부탁드립니다~~

@lamodadite lamodadite changed the title FEAT : 요구사항 구현, 테스트 코드 통과 확인 Be/main/jongmlee java lv1 Jan 7, 2025
Copy link
Contributor

@saewoo1 saewoo1 left a comment

Choose a reason for hiding this comment

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

예끼.. 커밋 상태가 무척 불순하지만 제출했다면 오케입니다!
쓸데없는 커밋은 더 혼란을 주지만, 까비 컨벤션을 읽고 따라만 한다면 당신은 이미 커밋 마스터

이전 단계에서는 책임 및기능분리를 보여주셨던 만큼, 이번 레벨에서도 잘 하실 것 같은데 팔만대장경 클래스라니.. 다소 아쉽읍니다..ㅠㅠ 나중에 리팩토링.. 해주실거죠?
이번 단계도 고생 많으셨습니다!!

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
Contributor

Choose a reason for hiding this comment

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

words의 길이도 봐야하고, 내부 요소도 하나하나 봐야하고.. 메서드마다 조건이 너무 많은 것 같다는 생각이 든다면,
다음 단계로는 제품과 명령어로 분리하는 방식은 어떨까요?!

Copy link
Author

Choose a reason for hiding this comment

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

빨리 해야한다는 압박 속에서 하다보니 가독성도 안좋고 책임분리도 안된 코드를 제출해 버렸네요.. 이제 보니 그냥 전부 갈아엎어야 할 수준인데 이럴거면 처음부터 시간 들이더라도 꼼꼼히 할걸 그랬습니다!! 리팩토링 꼭 해볼게요!!

Copy link
Contributor

Choose a reason for hiding this comment

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

출력부 분리 해다오..
addOrder라는 메서드는 이름만 봤을 땐 주문 추가만 할줄 알았는데, print까지 하고있었군여!

Copy link

@chyo1 chyo1 left a comment

Choose a reason for hiding this comment

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

입력, 출력문도 lv0 과제처럼 상수로 뺐으면 좋았을 것 같아요
과제 힌트에 Enum 내용이 있으니, enum을 활용해보는 것도 좋을 것 같습니다
수고하셨슴다~

Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

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

상품을 뺄 때 해당하는 상품을 모두 빼야 하니까 상품과 개수를 한 번에 관리할 수 있는 Map같은 자료구조를 사용하는 것도 좋을 것 같아요

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.

4 participants