-
Notifications
You must be signed in to change notification settings - Fork 122
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
경북대 BE_이동원_1주차 과제(1단계 ~ 3단계) #204
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트까지 작성하시느라 고생 많으셨습니다.
전반적으로 깔끔하고 좋은 코드입니다.
컨트롤러 단의 가독성 부분만 조금 더 챙겨지면 좋을 것 같습니다.
this.dataSource = dataSource; | ||
} | ||
|
||
@Bean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빈 등록을 config에서 중복적으로 하고 있는 것으로 보입니다.
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
|
||
@Controller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 해당 어노테이션을 통해 빈 등록이 되니 config에서는 지워주세요.
import org.springframework.web.bind.annotation.GetMapping; | ||
|
||
@Controller | ||
public class ErrorController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
목적에 따른 클래스 분리 좋습니다.
} | ||
|
||
// 모든 제품을 가져오는 엔드포인트 | ||
@GetMapping("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("") 는 지워주셔도 좋습니다.
public ResponseEntity<Product> putProduct(@RequestBody ProductForm form, @PathVariable Long id) { | ||
Product product = new Product(form); // ProductForm을 Product로 변환 | ||
Product result = repository.edit(id, product); | ||
if (result != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 처리가 보일러플레이트처럼 반복되는 것으로 보입니다.
global exception handler 의 도입을 추천드립니다.
@PostMapping(path = "", consumes = "application/json") | ||
public ResponseEntity<Product> postProduct(@RequestBody ProductForm form) { | ||
Product product = new Product(form); // ProductForm을 Product로 변환 | ||
Product result = repository.save(product); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컨트롤러 단에서 바로 로직이 실행되고 있습니다.
특정 클래스를 통해 controller에서 repository를 참조하지 않는 것을 추천드립니다.
} | ||
|
||
// 특정 ID의 제품을 수정하는 엔드포인트 | ||
@PutMapping(path = "/{id}", consumes = "application/json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consumes은 기본값을 확인해보고 생략해주셔도 좋습니다.
|
||
@Test | ||
@DisplayName("존재하지 않는 ID 삭제") | ||
public void delete_nonexistent() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 코드 아주 좋습니다.
// given
// when
// then
이와 같은 양식을 익혀둔다면 추후 테스트 작성시 용이합니다.
실무에서도 해당 양식을 주석으로 적어 통일성과 유지보수성을 높이고 있습니다.
conflict 도 resolve 부탁드립니다! |
No description provided.