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

feat:글 조회 수정 삭제 삽입 #46

Open
wants to merge 3 commits into
base: spring
Choose a base branch
from
Open

Conversation

sleeg00
Copy link
Collaborator

@sleeg00 sleeg00 commented Nov 2, 2022

Test구현 예정입니다. 예외처리 한 번 봐주시면 감사하겠습니다

[#8]

Test구현 예정입니다. 예외처리 한 번 봐주시면 감사하겠습니다

[#8]
public List<Post> search(@PathVariable String postAuthor, HttpServletResponse response){
try{
List <Post> post = postService.search(postAuthor);
if(!post.isEmpty()) { response.setStatus(HttpServletResponse.SC_OK);
Copy link

Choose a reason for hiding this comment

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

컨트롤러 단에서 예외 처리도 하나의 비즈니스 로직이라
서비스 단에서 exception throw를 하고 호출 메소드에서
try-catch 하는거를 추천!

List <Post> post = postService.search(postAuthor);
if(!post.isEmpty()) { response.setStatus(HttpServletResponse.SC_OK);
return post;
}
Copy link

Choose a reason for hiding this comment

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

API를 호출하는 입장에서 post가 존재하지 않으면
400으로 code 보는것은 글이 없는건지? 서버 내부 에러인건지 모호해서
not found error를 던져서
“글이 없다는것”을 알려주는게 좋을듯

//postService.update(postDto);
try{
Optional <Post> post = Optional.ofNullable(postService.save(postDto));
if(post.isPresent()){response.setStatus(HttpServletResponse.SC_OK);
Copy link

Choose a reason for hiding this comment

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

동일하게 exception call을 서비스단에서 하는것을 추천

@GetMapping("/delete/{postAuthor}") //삭제 기능
public void delete(@PathVariable String postAuthor, HttpServletResponse response){
try{
if(postService.delete(postAuthor)) response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
Copy link

Choose a reason for hiding this comment

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

이 부분에서 글 삭제가 정상적으로 처리되면
http 상태 코드를 400으로 던지는데
의도한 처리인지?

Copy link
Collaborator Author

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.

httpServletResponse 말고 responseEntity에서 https 로 상태 던집시다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 다음 기능구현때 사용해 보겠습니다!

if(postService.delete(postAuthor)) response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
}
catch (Exception e){
response.setStatus(HttpServletResponse.SC_OK);
Copy link

Choose a reason for hiding this comment

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

위와 동일하게 exception 시 200?

Copy link
Member

Choose a reason for hiding this comment

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

예외일때 200 던지는거 비추

private String postImage;

public void setPostId(Long postId) {
this.postId = postId;
Copy link

Choose a reason for hiding this comment

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

setter, getter 롬복 어노테이션 추천
그런데 실무에서는 lombok 사용에 대해 고민해봐야함..

import javax.persistence.Id;
import java.util.Date;

@Getter
Copy link

Choose a reason for hiding this comment

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

밑에 @DaTa 하나 만 달아도 됨
setter 빼고

public class Member {
@Id
@Column(name = "Id")
private String Member_id;
Copy link

Choose a reason for hiding this comment

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

카멜케이스로 변수명 짓는것을 추천!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어 저는 Post (DTO, DAO)만 건들여서 그쪽만 봐주실 수 있을까요?

Copy link
Member

@MoonSangWon MoonSangWon left a comment

Choose a reason for hiding this comment

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

lgtm 리뷰어를 백엔드 하는 친구들만 올립시다. !

완료했습니다.

[#8]
@sleeg00 sleeg00 closed this Nov 11, 2022
@sleeg00 sleeg00 reopened this Nov 11, 2022
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.

3 participants