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 추가 #1

Merged
merged 3 commits into from
Sep 10, 2024
Merged

회원가입 api 추가 #1

merged 3 commits into from
Sep 10, 2024

Conversation

5upportPark
Copy link
Collaborator

회원가입 api 추가
필수 값 validate 로직 적용

@5upportPark 5upportPark merged commit 42329ba into main Sep 10, 2024
1 check passed
Copy link

@f-lab-Tailor f-lab-Tailor left a comment

Choose a reason for hiding this comment

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

리뷰 남깁니다!


@RestController
@RequestMapping("/users")
@RequiredArgsConstructor

Choose a reason for hiding this comment

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

Constructor의 경우 lombok을 사용하기보다는 직접 작성 하는것이 좀 더 좋습니다!

}

@PostMapping
public Map<String, Object> registUser(@RequestBody @Validated UserDTO user, BindingResult bindingResult){

Choose a reason for hiding this comment

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

response를 객체를 사용하는 구조로 바꿔보는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ResponseEntity를 말씀해주신게 맞을까요?

private Long id;
@Column(name = "NAME")
private String name;
@Column(name = "GENDER")
private Integer gender;

Choose a reason for hiding this comment

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

gender를 INTEGER값을 이용한 이유가 있을까요? ENUM을 활용해보는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

평소에 코딩하던 대로 하다보니 Integer로 하게 된 것 같습니다. 해당 부분 Enum타입으로 수정해보겠습니다!

@Column(name = "CREATED")
private Instant created;
@Column(name = "UPDATED_BY")
private String updatedBy;
@Column(name = "UPDATED")
private Instant updated;

Choose a reason for hiding this comment

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

createdAt, updatedAt도 있으면 좋을 것 같습니다!

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.List;

@Service
@Transactional

Choose a reason for hiding this comment

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

service 전체에 transactional을 거는 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

service안에 있는 모든 메소드들에 transactional을 걸어주려고 이렇게 선언하게 되었습니다

}

public UserDTO insertUser(UserDTO userDTO){
UserDTO result = userRepository.save(userDTO.toEntity()).toDTO();

Choose a reason for hiding this comment

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

save에서 return하는 객체가 null일 경우엔 어떻게 될까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NullException이 발생할 것 같습니다ㅠ 해당 부분 null체크 로직 추가하도록 하겠습니다!

Choose a reason for hiding this comment

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

Optional.ofNullable(userRepository.save(userDTO.toEntity()).orElseThrow( new BussinessException());

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.List;

@Service
@Transactional
public class UserService {
@Autowired

Choose a reason for hiding this comment

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

Autowired vs Constructor 어떤게 다를까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

생성자를 이용하면 final 멤버 변수 초기화할 수 있고, 멤버 변수의 불변성도 지킬 수 있지만 Autowired는 멤버 변수 값을 재할당할 수 있어서 불변성이 지켜지지 않습니다

}

public UserDTO insertUser(UserDTO userDTO){
UserDTO result = userRepository.save(userDTO.toEntity()).toDTO();

Choose a reason for hiding this comment

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

Optional.ofNullable(userRepository.save(userDTO.toEntity()).orElseThrow( new BussinessException());

@@ -9,4 +9,5 @@
@Repository
public interface UserRepository extends JpaRepository<User, Long> {
public List<User> findAll();
public User save(User user);

Choose a reason for hiding this comment

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

Optional<User> save(User user);

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