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

[2주차] 기본 과제 & 심화 과제 제출 #4

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

Conversation

ddongseop
Copy link
Member

@ddongseop ddongseop commented Apr 18, 2023

4/18 21:12 기준 기본 과제만 완료한 상태이며, 심화 과제도 빠르게 제출하겠습니다!
4/18 22:34 기준 심화과제 (GET, POST, PUT, DELETE Method를 각각 사용하는 API 만들기) 완료하였습니다.

Copy link
Member

@jiyeoon00 jiyeoon00 left a comment

Choose a reason for hiding this comment

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

심화과제까지 수고하셨어요~!

private final PostService postService;

@PostMapping("/post")
public String upload(@RequestBody final UploadRequestDto request) {
Copy link
Member

Choose a reason for hiding this comment

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

저번부터 궁금했는데, 여기 프론트에서 받아오는 정보들을 매개변수로 받을 때 final로 하는 특별한 이유가 있나요?!
제 생각에는 final로 받아서 메소드 내에서는 변경이 불가능하도록? 안전장치를 해놓는건가 싶은데 동섭님 의견도 궁금하네요, 아 그리고 위 같은 경우에 항상 매개변수를 final로 선언하시는지 궁금해요!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 위 같은 경우에 final을 선언해본적이 없었는데, 팟장님 실습코드를 보고 final로 선언한 이유에 대해서 생각해보게 됐어요!
저두 받아온 request를 메소드 내에서 변경하게 되면 처음 받아온 request를 다시 참고하고 싶을 때 방법이 없기 때문에 final을 사용했다고 생각해요!

Choose a reason for hiding this comment

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

저는 저번 세미나때 팟장님이 final로 하는 이유가 혹시모를 변경을 막기 위해서라고 기억해요!

// 서비스 계층에서 게시물 아이디로 게시물을 찾는 메서드 호출
Post findPost = postService.findOne(postId);

return new PostResponseDto(findPost.getId(), findPost.getTitle(), findPost.getWriting(), findPost.getLike(), findPost.getDislike());
Copy link
Member

Choose a reason for hiding this comment

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

매개변수들이 아주 많네요,, 아예 Post를 PostResponseDto의 생성자로 넘겨서 만들어주는건 어떨까요...? 이렇게 되면 PostResponseDto -> Post를 의존하게 되는데, Post는 자주 변하지 않으니 괜찮을 거 같은데 동섭님 의견도 궁금하네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

우왕 제 생각에도 그렇게 하는게 훨씬 깔끔할 것 같네요! 바로 수정해보려구요 :)
Post에 의존하게 되는 부분은.. 제 생각에도 지금 같은 경우에는 괜찮다고 생각하는데, 상황에 따라서 의존하지 않도록 설계해야하는 경우도 있을 것 같아요!

List<PetResponseDto> petDtoList = new ArrayList<>();
for (Pet curPet : petList) {
petDtoList.add(new PetResponseDto(curPet.getId(), curPet.getName(), curPet.getSpecies(), curPet.getGender(), curPet.getAge()));
}
Copy link
Member

Choose a reason for hiding this comment

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

return petList.stream().map(pet -> new PetResponseDto(curPet.getId(), curPet.getName(), 
curPet.getSpecies(), curPet.getGender(), curPet.getAge())).collect(Collectors.toList());

이렇게 리스트 내 객체를 변환하고 싶을 때 for문을 돌면서 변환해주는 방법도 있고 위에 제가 쓴 코드처럼 자바 stream을 이용해서 변환할 수 있는 방식이 있어요! 저도 익숙지 않아서 좀 더 공부해봐야 할 것 같은데 동섭님도 한번 적용해보셔도 좋을 것 같습니다🙂

Copy link
Member Author

@ddongseop ddongseop Apr 20, 2023

Choose a reason for hiding this comment

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

우와 위에서 피드백 해주신것처럼 아예 Pet 객체를 PetResponseDto의 생성자로 넘겨주는 방식을 택했더니 피드백 해주신 stream을 사용하는 코드가 더 간단하게 변했어요!

return petList.stream().map(PetResponseDto::new).collect(Collectors.toList());

람다를 메소드 참조 방식으로 바꿀 수 있다고 Intellij가 알려줬는데, 어렵긴 하지만 굉장히 간단해지네요!
저도 stream 사용이 익숙치 않은데, 일부러라도 사용해보려고 노력해야겠습니다! 피드백 감사드려요~~

Choose a reason for hiding this comment

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

스트림..... 지연언니한테 하나 배워갑니다!!

Copy link
Member

Choose a reason for hiding this comment

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

우와 진짜 코드가 더 간단해지고 자주 사용할수 있을것 같아요 ..!! 배워갑니다!!

Copy link

@YuSuhwa-ve YuSuhwa-ve left a comment

Choose a reason for hiding this comment

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

바쁜 시험기간에 심화과제까지 끝낸 "J"

private final PostService postService;

@PostMapping("/post")
public String upload(@RequestBody final UploadRequestDto request) {

Choose a reason for hiding this comment

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

저는 저번 세미나때 팟장님이 final로 하는 이유가 혹시모를 변경을 막기 위해서라고 기억해요!

post = curPost;
}
}

Choose a reason for hiding this comment

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

[P3] 다른 서비스 로직도 마찬가지로, 매개변수(이경우는 postId)가 유효하지 않은 값일때의 에러분기 처리가 있으면 더 좋을것같아요

Copy link
Member Author

@ddongseop ddongseop Apr 20, 2023

Choose a reason for hiding this comment

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

public Post findOne(Long postId) {
        Post post = null;

        for (Post curPost : postList) {
            if (curPost.getId().equals(postId)) {
                post = curPost;
            }
        }

        return post;
}

이 코드에서 postId가 null인 경우와 postId와 일치하는 값이 실제로 postList에 없는 경우를 한번에 처리했다고 생각하는데,
(postId가 null이면 curPost의 Id 값과 일치하는 경우가 발생하지 않고, 따라서 return 값인 post가 초기값인 null이 유지되어 return되므로)
이렇게 하는것보다 postId가 null인 경우를 따로 빼주는게 좋을까?

Copy link
Member

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.

맞네..... 내가 짠 코드를 다른 사람이 이해하기 쉽게 짜는 습관..! 기르기 어렵지만 길러볼게!

return petService.findAll();
}

//수정을 위한 DTO에는 id 값도 넘어와야 하므로 별도의 DTO 사용

Choose a reason for hiding this comment

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

좋아요 좋아요. Dto를 response, request 별로 깔끔하게 분리했네요! 저는 맨날 dto를 그때 그때 추가하는 편이라서 이렇게 나름의 규칙을 가지고 dto를 만드는걸 배워갑니다~

List<PetResponseDto> petDtoList = new ArrayList<>();
for (Pet curPet : petList) {
petDtoList.add(new PetResponseDto(curPet.getId(), curPet.getName(), curPet.getSpecies(), curPet.getGender(), curPet.getAge()));
}

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants