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 구축 코드 #17

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

Conversation

yuna83
Copy link

@yuna83 yuna83 commented Jun 5, 2024

LuckyNumberController: 숫자 1~9 중 랜덤으로 숫자 1개를 반환
UserController: 사용자 등록, 조회, 삭제

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.

LGTM 다음 과제때는 데이터베이스 연동, 서비스 코드를 추가할 수 있도록 해보겠습니다. 고생하셨습니다~! 커밋 이름이 활기차네요

private String email;
private String password;

// Getters and Setters
Copy link
Contributor

@coke98 coke98 Jun 23, 2024

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.

사용하지 않는 메서드는 만들지 않는게 좋습니다~!

@GetMapping("/{email}")
public User getUserByEmail(@PathVariable String email) {
Optional<User> user = users.stream().filter(u -> u.getEmail().equals(email)).findFirst();
return user.orElse(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

optional을 활용한 점은 좋았습니다만, 응답에 null을 주는게 좋을지 고민해봐도 좋을 것 같습니다.
null로 리턴값을 처리하는 것보다. default 값이나, 상태코드로 상태를 알리는 건 어떨까요?
추후 프로젝트에서는 orElseThrow를 사용해서 예외를 던져주는 방식도 있으니 활용해보셔도 좋을 것 같아요

Copy link
Member

@mjj111 mjj111 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ㅎㅎㅎ👍

Iterator<User> iterator = users.iterator();
while (iterator.hasNext()) {
User user = iterator.next();
if (user.getEmail().equals(email)) {
Copy link
Member

Choose a reason for hiding this comment

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

getEmail로 꺼내와서 비교하는것 보다는 user.sameEmail() 로 하나 만들어서 적용하는 것도 좋을 것 같아요

private String email;
private String password;

// Getters and Setters
Copy link
Member

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

Successfully merging this pull request may close these issues.

3 participants