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

임채현 API 구축 코드 #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dolchae
Copy link

@Dolchae Dolchae commented Jun 4, 2024

  1. SeasonController를 통해 사계절 중 랜덤 하나의 계절 가져오기
  2. UserController를 통해 User name 추가/삭제/반환 구현

Copy link
Contributor

@coke98 coke98 left a comment

Choose a reason for hiding this comment

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

처음 짜보느라 생소했을텐데 고생하셨어요~! RESTFUL URL 네이밍에 대해 한번 더 고민해봐도 좋을 것 같습니다

public class UserController{
private List<User> users = new ArrayList<>();

@GetMapping("/get")
Copy link
Contributor

Choose a reason for hiding this comment

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

이미 GET메서드가 URL에서 명시한 자원을 가져온다라는 의미가 있기에
GET메서드와 중복되게 /user/get을 써야할지 한번 고민해봐도 좋을 것 같아요

return users;
}

@PostMapping("/add")
Copy link
Contributor

Choose a reason for hiding this comment

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

Post와 Delete도 마찬가지로 이미 행동에 대한 의미를 내포하고 있어요
url은 자원 명시로 끝나면 더 restful하게 만들 수 있을 것 같아요.
@PostMapping()으로 빈칸으로 두어도 괜찮을 것 같아요

return "User " + name+" 추가되었습니다.";
}

@DeleteMapping("/remove")
Copy link
Contributor

Choose a reason for hiding this comment

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

User를 Delete할때 가끔 겪는 문제중 하나가 Delete를 쓴다면, RequestBody를 쓰지 못한다는 점인데요.

가끔 삭제전 확인을 해야하는 민감한 정보(비밀번호 등)가 있을 경우, requestBody를 쓰는 편이 Param보다는 낫습니다.
(이 이유는 한번 찾아보시면서 공부해보시면 좋을 것 같습니다. 왜 그런지 답변을 달아주면 다른 분들도 다른 분들도 확인이 가능할 것 같아요)

그렇다면 User를 삭제함에도 불구하고 Delete대신 Post를 쓰는 등의 해결책이 있을텐데..
메서드에 이유가 있다면 유연하게 사용할 수도 있을지 엄격하게 Restful하게 적용할지 고민해봐도 좋을 것 같았어요

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