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

전남대 BE_정호성_1주차 과제(2단계) #168

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

Conversation

yoong-saks
Copy link

thymeleaf를 이해하려고 노력했습니다.
기존에 Controller 내부에 있는 HashMap을 분리해서, 비즈니스 로직 Service 클래스를 만들어 기능을 분리하였습니다

createProduct의 기능 구현에 혼동이 온 것 같습니다.. view에 기본값의 product를 생성해서 보내는 것이 맞는 지 모르겠습니다

Copy link

@soominsohn soominsohn left a comment

Choose a reason for hiding this comment

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

안녕하세요 호성님! 리뷰어 앤지입니다.
Step2 미션도 잘 해주셨네요.
Step1과 마찬가지로 작성해주신 코드에 대한 질문과 반영해주시면 좋을 부분들에 대해 리뷰 남겼습니다.
최종 머지는 Step3로 하겠습니다!

리뷰 보시고 편하게 답변 달아주세요!

Comment on lines +21 to +22
@WebMvcTest(ProductController.class)
public class ProductControllerTest {

Choose a reason for hiding this comment

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

Step2 테스트가 전부 깨지고 있어요! 확인하고 고쳐볼까요?
미션을 제출하기전에 테스트 동작여부를 체크하고 제출하시면 더욱더 좋습니다 :)

Comment on lines +36 to +40
@GetMapping("/create")
public String createProductForm(Model model) {
model.addAttribute("product", new Product(1L, "name", 0L, "image.url"));
return "create";
}

Choose a reason for hiding this comment

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

view에 기본값의 product를 생성해서 보내는 것이 맞는 지 모르겠습니다

라고 질문주신 코드 인 것 같아요. 덕분에 테스트하기 수월했습니다 👍
현재 서버에서 model에 기본값을 생성해주고 있네요!

기본값을 설정해주는 곳으로는 서버, 클라이언트가 있습니다.
서버에서 생성해주게되면 다양한 클라이언트에서 동일한 기본값을 일관되게 받을수 있구요,
클라이언트에서 생성해주게되면 클라이언트에서 상황에 따라 동적으로 기본값을 설정할 수 있습니다.
상황에 따라서 적절한 곳에 기본값을 설정해주면 되는데, 현재상황에서는 서버에서 잘 생성하신 것 같아요 💯

private final AtomicLong incrementCounter = new AtomicLong(1); // ID를 관리할 변수

public List<Product> getProducts() {
return List.copyOf(products.values());

Choose a reason for hiding this comment

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

List.copyOf(products.values());를 이용해 컬렉션을 복사하여 반환하고 있군요!
불변을 만들기 위해 컬렉션을 복사하는 방법에는 어떤 것들이 있고, 많은 방법 중 List.copyOf를 사용하신 이유는 무엇일까요?

@@ -0,0 +1,68 @@
package gift.web.dto;

Choose a reason for hiding this comment

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

AdminController 클래스가 dto 패키지 하위에 있네요!
여기에 위치하게 된 이유가 있을까요?

Comment on lines +1 to +4
package gift.web.dto;

public record Product(Long id, String name, Long price, String imageUrl) {
}

Choose a reason for hiding this comment

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

Product 클래스가 dto 패키지 하위에 있네요!
호성님이 생각하신 dto 패키지의 역할은 무엇일까요?

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