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

3차 세미나 : 기본과제 #6

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

3차 세미나 : 기본과제 #6

wants to merge 7 commits into from

Conversation

05AM
Copy link
Member

@05AM 05AM commented May 4, 2023

구현 내용

3차 세미나 실습 코드 + @

  • raw JPA을 체험하기 위해 Entity alarm, alarm repository 추가
  • Dto와 entity를 mapping하기 위한 mapstruct 추가
  • 추가
    • AlarmRepository
    • AlarmController
    • AlarmService
    • AlarmResponseDto
    • UserMapper
    • AlarmMapper

Copy link

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

늦게 리뷰 달아서 죄송합니다 !!! 🙇‍♂️🙇‍♂️🙇‍♂️
작업하시느라 수고하셨습니다 ~ !

Comment on lines +18 to +20

// 알람 생성
@PostMapping("")
Copy link

Choose a reason for hiding this comment

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

Suggested change
// 알람 생성
@PostMapping("")
// 알람 생성
@PostMapping

이런 상황에서 소괄호를 없애도 됩니다..!!

Comment on lines +9 to +14
@Entity
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
@Builder
Copy link

Choose a reason for hiding this comment

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

코드를 한번 쭉 봤을 때 setter를 사용하지 않는 것 같아 보입니다..!
@Setter 어노테이션을 사용할 때만 붙이는 것이 어떨까요..?
왜냐하면 해당 어노테이션으로 인해 런타임 시에 불필요한 코드(set~~)가 추가되기 때문입니다..!

Comment on lines +9 to +15
@Mapper
public interface UserMapper {
UserMapper INSTANCE = Mappers.getMapper(UserMapper.class);

@Mapping(target = "userId", expression = "java(user.getId())")
UserResponseDto ToDto(User user);
}
Copy link

Choose a reason for hiding this comment

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

오오... 처음 보는 어노테이션이네요..!
해당 어노테이션에 대해서 공부해 봐야겠네요..!! 👍

Comment on lines +39 to +44
ArrayList<AlarmResponseDto> alarmResponseDtoList = new ArrayList<>();
for(Alarm alarm : alarmList) {
alarmResponseDtoList.add(
AlarmMapper.INSTANCE.ToDto(alarm)
);
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
ArrayList<AlarmResponseDto> alarmResponseDtoList = new ArrayList<>();
for(Alarm alarm : alarmList) {
alarmResponseDtoList.add(
AlarmMapper.INSTANCE.ToDto(alarm)
);
}
alarmList.stream()
.map(AlarmMapper.INSTANCE::ToDto)
.collect(Collectors.toList());

이 map 형식이 잘 될지는 저도 확신은 안서지만 stream을 사용한다면 이쁘게 매핑할 수 있을 것 같습니다!

Comment on lines +10 to +31
@Repository
@RequiredArgsConstructor
public class AlarmRepository implements CustomRepository<Alarm> {
private final EntityManager em;
@Override
public void save(Alarm alarm) {
em.persist(alarm);
}
@Override
public Optional<Alarm> findById(Long id) {
Alarm alarm = em.find(Alarm.class, id);
return Optional.ofNullable(alarm);
}

public List<Alarm> findByUserId(Long userId) {
List<Alarm> result = em.createQuery("select a from Alarm a where a.user.id = :userId", Alarm.class)
.setParameter("userId", userId)
.getResultList();

return result;
}
}
Copy link

Choose a reason for hiding this comment

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

오오.. 이런 방식 처음이라서 신기하네요!!
혹시 Jpa Repository를 사용하지 않고
Custom Repository를 추상화한 후 구현체를 만든 이유가 있을까요?!?

Copy link
Member

Choose a reason for hiding this comment

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

저도 궁금합니다!
따로 구현할 필요없이 메서드명만 작성하면 편하게 사용 가능한 JPA를 사용하지 않고
직접 구현하신 이유가 있을까요?

Copy link
Member

@PgmJun PgmJun left a comment

Choose a reason for hiding this comment

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

수고하셨숩니다~!!!

AlarmMapper INSTANCE = Mappers.getMapper(AlarmMapper.class);

@Mapping(target = "userId", expression = "java(alarm.getUser().getId())")
AlarmResponseDto ToDto(Alarm alarm);
Copy link
Member

Choose a reason for hiding this comment

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

메서드의 첫 글자는 카멜케이스 문법을 따라 소문자로! 적는 것을 권장드립니다!

Comment on lines +10 to +31
@Repository
@RequiredArgsConstructor
public class AlarmRepository implements CustomRepository<Alarm> {
private final EntityManager em;
@Override
public void save(Alarm alarm) {
em.persist(alarm);
}
@Override
public Optional<Alarm> findById(Long id) {
Alarm alarm = em.find(Alarm.class, id);
return Optional.ofNullable(alarm);
}

public List<Alarm> findByUserId(Long userId) {
List<Alarm> result = em.createQuery("select a from Alarm a where a.user.id = :userId", Alarm.class)
.setParameter("userId", userId)
.getResultList();

return result;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

저도 궁금합니다!
따로 구현할 필요없이 메서드명만 작성하면 편하게 사용 가능한 JPA를 사용하지 않고
직접 구현하신 이유가 있을까요?


@Entity
@Getter
@Setter
Copy link
Member

Choose a reason for hiding this comment

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

엔티티 클래스에는 불변성을 보장하기 위해 Setter를 사용하지 않는 것을 권장드립니다!

return ApiResponseDto.success(SuccessStatus.SIGNUP_SUCCESS, userService.create(request));
}

@GetMapping("/user/{userId}")
Copy link
Member

Choose a reason for hiding this comment

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

@RestController
@RequiredArgsConstructor
@RequestMapping("/user")
public class UserController {
    private final UserService userService;

    @PostMapping("/signup")
    @ResponseStatus(HttpStatus.CREATED)
    public ApiResponseDto<UserResponseDto> create(@RequestBody @Valid final UserRequestDto request) {
        return ApiResponseDto.success(SuccessStatus.SIGNUP_SUCCESS, userService.create(request));
    }

    @GetMapping("/{userId}")

공통 path는 이렇게 묶어서 사용이 가능합니다!

}

// 유저 아이디로 알람 가져오기
@GetMapping("")
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도
@GetMapping("") 보단 @GetMapping로 사용하는게 깔끔하다고 생각이 됩니다!

Copy link
Member

@PgmJun PgmJun left a comment

Choose a reason for hiding this comment

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

수고하셨숩니다~!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants