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코드 작성 #14

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

Conversation

juchan2002
Copy link

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.

고생하셨습니다 ㅎㅎ 👍

public class TaskController {
private final TaskService taskService;

@Autowired
Copy link
Member

Choose a reason for hiding this comment

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

음?... 이건 조금 이상하네요... 인터넷에 생성자 주입으로 Spring관련해서 검색해보시면 좋을 것 같아요.

import java.util.List;
import java.util.Optional;

@Controller
Copy link
Member

Choose a reason for hiding this comment

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

@responsebody 보다는
해당 애너테이션을 @RestController 로 수정하면 됩니다 !

private String password;

// Constructors
public User() {}
Copy link
Member

Choose a reason for hiding this comment

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

해당 생성자는 왜 만드셨을까요?

this.userRepository = new UserRepository();
}

public User createUser(String name, String email, String password) {
Copy link
Member

Choose a reason for hiding this comment

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

DTO로서 하나의 객체로 받는게 더 좋을 것 같아요~!

import org.junit.jupiter.api.Test;
import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest
Copy link
Member

Choose a reason for hiding this comment

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

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


@PostMapping
@ResponseBody
public Task createTask(@RequestParam String title, @RequestParam String description, @RequestParam LocalDate dueDate, @RequestParam boolean status) {
Copy link
Member

Choose a reason for hiding this comment

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

Post라면 body에 담아서 보내는게 어떨까요?

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 고생하셨습니다! 유일하게 이슈와 PR 연동해서 작성해주신 점도 좋았어요. 프로젝트에서는 이슈, PR 템플릿도 지정하여 협업할 수도 있으니 활용해보셔도 좋을 것 같아요.

this.status = status;
}

// Getters and setters
Copy link
Contributor

Choose a reason for hiding this comment

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

자동 생성이라면 사용하지 않는 게터 세터는 삭제하는 편이 좋아요. 캡슐화와 정보 은닉에서 객체지향의 장점을 어떻게 살릴 수 있을지 고민해보면 좋을 듯 합니다

private boolean status;

// Constructors
public Task() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

기본 생성자가 필요한 이유, 해당 코드에서 기본 생성자를 직접 명시해준 이유가 무엇일까요?


public class UserRepository {
private List<User> users = new ArrayList<>();
private AtomicLong nextId = new AtomicLong(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

데이터베이스 연동없이 레포지토리 계층 만들기위해 필드로 임시로 메모리 저장을 시도해주셨네요👍

return user.orElse(null);
}

@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.

patch put 모두 수정에 사용되는 메서드인데요, 둘은 어떤 차이가 있을까요? 그렇다면 현재는 어떤 메서드를 사용하는 것이 좀 더 적절할까요? 답변 남겨주시면 감사하겠습니다!


@PostMapping
@ResponseBody
public User createUser(@RequestParam String name, @RequestParam String email, @RequestParam String password) {
Copy link
Contributor

Choose a reason for hiding this comment

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

requestParam 대신 post 요청에서는 requestBody를 사용하는 편이 더 적절할 것 같아요.
보통 코딩을 하다 파라미터가 3개 이상 나열되기 시작한다면(책 - 클린코드) 다른 방법이 없을지 고민해보면 좋을 것 같아요.
바디를 사용하면 바인딩을 통해 원하는 객체(예: UserCreateRequest)로 곧바로 변환해 활용할 수 있습니당

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.

정주찬 api 코드 작성
3 participants