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 구현 코드입니다! #16

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

Conversation

o-jeong
Copy link

@o-jeong o-jeong commented Jun 5, 2024

No description provided.

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
예외처리 등 여러가지 시도를 해보신 점이 좋았습니다~! 다만,, 과제로 드렸던 노션 페이지가 없어 실제로 동작하는 지 확인이 불가능하네요!

확인하시는대로 postman으로 요청 후 결과 링크 남겨주시면 감사하겠습니다!

}

@PostMapping
public Student createStudent(Student student){
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 어노테이션 없이 바인딩이 되던가요?👀

Copy link
Author

Choose a reason for hiding this comment

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

처음 postman으로 확인할때 그냥 널값이 반환되었는데 제대로 확인안해보고 그냥 넘어가버렸습니다... ㅎㅎ.. @RequestBody로 바인딩해서 다시 postman으로 확인했습니다!!!

import java.util.NoSuchElementException;

@RestController
@RequestMapping("api/students")
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.

취향이지만 써주신 것처럼 api로 Url 구분 두는 방법도 좋은 방법중에 하나긴 합니다~
더 활용해서 api/v1/students 와 같이 버전을 명시하기도 한답니당


@PutMapping("/{id}")
public Lecture addLecture(@PathVariable Long id, @RequestBody Lecture lecture){
Student student = students.stream().filter(s -> s.getId().equals(id)).findFirst().orElseThrow(()->new NoSuchElementException());
Copy link
Contributor

Choose a reason for hiding this comment

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

옵셔널 객체에서 예외처리로 시도한 점이 좋았습니다~!
해당 예외 상황을 재현할 경우 클라이언트에서는 어떤 식으로 나타나는지 확인해보셨나요?

Copy link
Author

Choose a reason for hiding this comment

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

존재하지 않는 학생의 id에 강의를 추가하려는 경우
"timestamp": "2024-06-28T07:12:22.448+00:00",
"status": 500,
"error": "Internal Server Error"
위와 같은 응답을 받습니다.

return id;
}
public void setId(){
this.id = id;
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분,,, 뭔가 문제가 있을 것 같지 않나요?✨ 어떤 문제인지는 말씀드리지 않겠습니다! 답변으로 남겨주세요~

Copy link
Author

Choose a reason for hiding this comment

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

public void setId(Long id){
    this.id = id;
}

매개변수를 깜빡했습니다.. ! ㅎㅎ

return student;
}

@PutMapping("/{id}")
Copy link
Contributor

Choose a reason for hiding this comment

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

/{id}/lectures 의 경로로 하위자원을 명시하면 좀더 명확하지 않을까 싶네요 :)

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.

고생하셨습니다 ㅎㅎ👍

return lectures;
}

@PostMapping
Copy link
Member

Choose a reason for hiding this comment

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

Lecture은 무엇으로 받는걸까요? Body값으로 받는다면 @RequestBody를 쓰면 됩니다~

import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest
class GdscApplicationTests {
Copy link
Member

Choose a reason for hiding this comment

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

테스트 코드가 없어서 아쉽네요 ㅠㅠ

@o-jeong
Copy link
Author

o-jeong commented Jun 28, 2024

https://www.notion.so/4-dafb40c006b847c696f691c02eec5bc7?pvs=4

노션 페이지 링크입니다!!!

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