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

부산대 BE_이영준_2주차 과제 step 1 #276

Open
wants to merge 21 commits into
base: 20jcode
Choose a base branch
from

Conversation

20jcode
Copy link

@20jcode 20jcode commented Jul 4, 2024

안녕하세요!

2주차 wishlist step1 과제 PR 입니다.

몇몇 궁금한 점이 있는데, PR과 동시에 조언을 구하고자 합니다.

  1. 클라이언트와 소통하는 과정에서 DTO가 용도에 맞게 있는 것이 좋은지? 하나만 있는 것이 좋은지?
    제 생각으로는 공통적인 포멧 하나를 다양하게 활용할 수 있다면, 협업하는 사람들의 귀찮음이 줄어들어서 좋을 것
    같다는 생각을 해보았습니다.

  2. 관심사 분리? 를 어떠한 방식으로 문서화할 수 있을지요?
    예를 들어 "카카오" 글자에 대한 예외를 발생시키는 것은 서비스계층에서 책임지는 것이 맞다고 생각되는데,
    만약 6개월 뒤에 저 혹은 타인이 이 부분에 대해 추가적인 작업을 진행하고자 한다면, 이를 어떤식으로 문서화하여 남겨두면 빠르게 필요한 부분을 확인할 수 있을까요?
    빠르게 확인해서 필요한 부분에

20jcode added 21 commits July 1, 2024 15:58
test를 위해서 추가
컨트롤러에서 해당 부분에 대해서 @Valid 구현 필요
경로 변경한 것에 대해서 정상 동작 확인
전역적으로 예외 처리를 위한 Advisor 추가

예외관련 내용을 담은 ExceptionResponseDTO 추가
Copy link

@cmst-steve cmst-steve left a comment

Choose a reason for hiding this comment

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

클라이언트와 소통하는 과정에서 DTO가 용도에 맞게 있는 것이 좋은지? 하나만 있는 것이 좋은지?
제 생각으로는 공통적인 포멧 하나를 다양하게 활용할 수 있다면, 협업하는 사람들의 귀찮음이 줄어들어서 좋을 것
같다는 생각을 해보았습니다.

공통적인 포맷이 있는것은 협업과정에서 매우 편리합니다.
정해놓은 응답형식에 맞게 클아이언트에게 데이터를 내려주면 클라이언트에서도 진행하기 수월합니다.

다만 " DTO가 용도에 맞게 있는 것이 좋은지? 하나만 있는 것이 좋은지?"이 질문과는 전혀 별개인 것 같습니다. 하나만 있다는게 모든 응답을 하나만으로 내려주겠다는 것 같은데, 이는 불필요한 데이터를 내려주어 오히려 자원을 낭비하게됩니다.

관심사 분리? 를 어떠한 방식으로 문서화할 수 있을지요?
예를 들어 "카카오" 글자에 대한 예외를 발생시키는 것은 서비스계층에서 책임지는 것이 맞다고 생각되는데,
만약 6개월 뒤에 저 혹은 타인이 이 부분에 대해 추가적인 작업을 진행하고자 한다면, 이를 어떤식으로 문서화하여 남겨두면 빠르게 필요한 부분을 확인할 수 있을까요?

가장 좋은 방법은 코드 자체로 잘 읽히게 두는 것입니다. 코드 자체로 이해할 수 있는 수준으로 잘 작성해두면 별도의 문서화나 주석없이 설명이 가능합니다.

다만 코드로만 설명이 부족한 경우 주석으로 남겨놓으면 되는데 영준님이 우려한 부분은 잘 짜여진 코드만으로 설명이 충분하다고 생각합니다.

이와 관련하여 클린코드라는 키워드로 검색하고 학습하시는거 권장드립니다 :)

@@ -0,0 +1,12 @@
package gift.dto;

public class ExceptionResponseDTO {

Choose a reason for hiding this comment

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

Response라고 이름을 정했으면 DTO는 제외해도 될 것 같아요~


//새로운 상품 추가
@Override
public void create(ProductDTO prod) {

Choose a reason for hiding this comment

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

불필요한 축약어는 사용하지않는게 좋습니다. productDto나 product로 표기하여 자연스럽게 읽히도록하면 좋을 것 같습니다 ~

Comment on lines +25 to +28
for (var product : products) { //DTO로 전환
productDTOList.add(new ProductDTO(product));
}

Choose a reason for hiding this comment

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

for문 대신에 productDTOList에서 stream.map함수를 이용하여 사용하는건 어떨까요?
stream은 무엇이고 어떠한 기능이 있을지 한번 학습해보시면 좋을 것 같습니다 :)

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