Skip to content

feat/week3#23

Open
mongdmin wants to merge 8 commits intoApptiveDev:mainfrom
mongdmin:week2
Open

feat/week3#23
mongdmin wants to merge 8 commits intoApptiveDev:mainfrom
mongdmin:week2

Conversation

@mongdmin
Copy link

@mongdmin mongdmin commented Nov 13, 2025

변경점 👍

코드리뷰 반영 수정(dto: class -> record / transaction 이용 / validation 이용 / 단방향 구성)
like 구현
테스트코드 추가

테스트 💻

image image image

Copy link
Contributor

@sinsehwan sinsehwan 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 +27 to +28
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "member_id")
Copy link
Contributor

Choose a reason for hiding this comment

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

현재 로직상 Comment는 Member정보가 항상 존재하기에 @ManyToOne, @JoinColumn에 제약조건을 더 걸어도 될 것 같습니다! 엔티티를 통해 DB 테이블 스키마를 간접적으로 정의한다고 생각하시면 될 것 같습니다.

@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(name = "member_id", nullable = false)

Comment on lines +40 to +45
public Long getId() { return id; }
public String getContent() { return content; }
public String getUsername() { return username; }
public LocalDateTime getCreatedAt() { return createdAt; }
public Task getTask() { return task; }
public Member getMember() { return member; }
Copy link
Contributor

Choose a reason for hiding this comment

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

@Setter는 지양해야 하지만 @Getter 까지는 사용해도 될 것 같아요

Comment on lines +23 to +24
@PathVariable Long taskId,
@Valid @RequestBody CommentRequest req) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Valid로 입력에 대한 검증을 수행하는군요! 좋습니다

Comment on lines +52 to +55
return commentRepo.findByMemberId(memberId)
.stream()
.map(CommentResponse::new)
.toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

스트림과 람다식 형식으로 코드 깔끔하게 잘 작성해주셨네요! 좋습니다

Comment on lines +8 to +10
Optional<Member> findByUsername(String username);

List<Member> findByUsernameContaining(String keyword);
Copy link
Contributor

Choose a reason for hiding this comment

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

username을 SELECT 조건문으로 사용하는 쿼리를 자주 쓸 것 같네요! username에 인덱스 걸면 좋을 것 같아요

@Table(name = "tasks")
public class Task {

public enum Status { TODO, IN_PROGRESS, DONE }
Copy link
Contributor

Choose a reason for hiding this comment

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

Enum 객체는 별도 클래스로 분리해서 관리해도 좋을 것 같습니다!

Comment on lines +18 to +26
@SpringBootTest
@Transactional
class CommentRepositoryTest {

@Autowired
private CommentRepository commentRepository;
@Autowired
private MemberRepository memberRepository;
@Autowired
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트코드도 이번에 작성해주셨네요! 좋습니다 그런데 사실 RepositoryTest보다는 Service, Controller 계층 테스트 코드를 우선적으로 작성해보시는 걸 추천드립니다! 왜냐하면 Repository는 현재 충분히 검증된 도구인 JPA를 사용하기에 문제가 없을 확률이 높기 때문입니다.

Comment on lines +3 to +5
public class TaskNotFound extends RuntimeException {
public TaskNotFound(Long id) { super("Task not found: " + id); }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

커스텀 예외 추가하셨네요! 좋습니다 이러면 나중에 전역 예외 처리 시 해당 예외만 별도로 처리할 수 있습니다

Comment on lines +7 to +10
String title,
String description,
LocalDate dueDate,
Integer priority,
Copy link
Contributor

Choose a reason for hiding this comment

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

이쪽 요청 DTO에도 validation 적용해보면 좋을 것 같습니다! 저는 RequestDto의 경우 일단 Validation을 적용하는 편입니다.

Comment on lines +27 to +31
Task task = taskRepo.findById(taskId)
.orElseThrow(() -> new IllegalArgumentException("Task not found"));

Member member = memberRepo.findById(req.memberId())
.orElseThrow(() -> new IllegalArgumentException("Member not found"));
Copy link
Contributor

Choose a reason for hiding this comment

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

예외처리 꼼꼼하게 처리해주시는 것 좋습니다! 이건 당장 적용할 필요는 없지만 orElseThorw() 관련 중복 코드를 좀 더 줄이고 싶이시다면 순수하게 값을 DB에서 조회하고 예외처리 수행하는 것 까지만 책임을 지는 서비스 계층(저는 보통 LowService 또는 RawService 정도로 이름지어서 사용합니다.)을 하나 더 둬서 Service 계층이 Repository 계층을 참조하지 않고 대신 예외처리까지 완료된 LowService 계층만 사용하는 형식으로 중복 코드를 줄이는 방법을 도입할 수도 있습니다.

// 예시
@Service
@RequiredArgsConstructor
@Transactional
public class DiaryLowService {
    private final DiaryRepository diaryRepository;

    public DiaryEntity saveDiary(DiaryEntity diary) {
        return diaryRepository.save(diary);
    }

    @Transactional(readOnly = true)
    public DiaryEntity findDiaryById(Long diaryId) {
        return diaryRepository.findById(diaryId)
                .orElseThrow(() -> new NotFoundEntityException(ExceptionCode.NOT_FOUND_DIARY.getDescription()));
    }

    public void updateDiary(DiaryEntity diary, DiaryUpdateDto updateDto) {
        diary.update(
                updateDto.musicTitle(),
                updateDto.artist(),
                updateDto.albumImageUrl(),
                updateDto.videoUrl(),
                updateDto.content(),
                updateDto.scope(),
                updateDto.duration(),
                updateDto.totalDuration(),
                updateDto.start(),
                updateDto.end()
        );
    }

    public void deleteDiary(DiaryEntity diary) {
        diaryRepository.delete(diary);
    }

    //...

}

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