Skip to content

Conversation

@hellmir
Copy link

@hellmir hellmir commented Jul 26, 2025

📋 작업 내용

  • 회원 도메인 리팩터링
  • 리팩터링에 따른 일부 테스트 코드 추가 및 수정

✅ 체크리스트

  • 테스트를 완료했나요?
  • 코드 컨벤션을 지켰나요?
  • 관련 문서를 업데이트 했나요?

hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 36d823f to 0c0c7ac Compare July 27, 2025 04:16
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 0c0c7ac to f98df32 Compare July 27, 2025 04:29
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from f98df32 to bea1fd7 Compare July 27, 2025 04:31
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from bea1fd7 to 277c035 Compare July 27, 2025 04:34
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 277c035 to 8a385c1 Compare July 27, 2025 04:49
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 8a385c1 to 0040463 Compare July 27, 2025 05:03
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 0040463 to c899dc8 Compare July 27, 2025 05:30
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from c899dc8 to fa1ca22 Compare July 27, 2025 05:33
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from fa1ca22 to 6e4dd57 Compare July 27, 2025 05:35
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch 2 times, most recently from d012123 to 87ae483 Compare July 27, 2025 06:37
hellmir added a commit to hellmir/zto that referenced this pull request Jul 27, 2025
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
- 엔티티 공용 멤버 관리를 위한 BaseGeneralEntity 클래스 추가
- 기존의 deleteAt 기반 논리적 삭제 로직을 deleteYn 기반으로 변경
- 기타 개선사항 고도화
@hellmir hellmir force-pushed the feature/SHB/250726/onboarding branch from 87ae483 to 92cc689 Compare July 27, 2025 07:03
Copy link
Collaborator

@rudeh1253 rudeh1253 left a comment

Choose a reason for hiding this comment

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

너무 훌륭하네요. 고개를 연신 끄덕이면서 코드를 읽었습니다. 기존 코드를 잘 이해하시면서도 기존 코드의 문제점을 잘 캐치하고 리팩토링해 주신 모습에서 역량이 매우 뛰어나시다는 점을 감지할 수 있었습니다. 여러모로 부족한 점이 많은 제가 잘 따라갈 수 있을지 모르겠지만, 같이 팀으로 프로젝트를 수행한다면 저희 코드의 질이 현격하게 좋아질 뿐만 아니라 저도 배우는 게 너무 많을 것 같아서 너무 좋을 것 같습니다.
정말 많은 것을 배울 수 있는 PR이었습니다. 군데군데 변경 사유를 주석으로 남겨주셔서 이해하기가 매우 수월했습니다. 이점 너무나도 감사드립니다. 많은 질문을 드리고 싶었지만, 정성스럽게 주석을 달아주셔서 궁금한 점이 생겨도 바로바로 해소되었어요.
나중에 직접 만나서 같이 이야기를 나눠 보고 싶은 마음도 듭니다.
다시 한 번 정성스러운 양질의 PR 감사드립니다!


public LocalDateTime getUpdatedAt() {
// 변경 사유: 자신의 날짜는 여기저기서 꺼내 볼 수 있게 하는 것보다 스스로가 관리하는 것이 객체지향의 원칙에도, DDD의 지향점에도 부합한다고 생각됩니다.
protected LocalDateTime getUpdatedAt() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋은 의견 감사합니다

package com.codezerotoone.mvp.domain.common;

public class EntityConstant {
public static final String BOOLEAN_DEFAULT_FALSE = "boolean default false";
Copy link
Collaborator

Choose a reason for hiding this comment

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

오라클처럼 Boolean 타입을 제공하지 않는 DB를 상정한다면, DB에서 BOOLEAN 타입을 사용하는 건 DB 선택의 폭이 좁아지지 않을까요? 아니면 DB 선택의 폭을 좁히더라도 boolean 타입을 사용함으로써 얻는 이점이 더 클까요?
효빈님 의견은 어떠신가요?

Copy link
Author

Choose a reason for hiding this comment

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

예를 들어 오라클을 사용하기로 하는 경우에, 상수로 선언했으므로

public static final String BOOLEAN_DEFAULT_FALSE = "number(1) default 0";

로 한 내용물만 바꾸면 되기 때문에 거의 수정이 필요 없을 것으로 보입니다.

Comment on lines +19 to +23
@Column(nullable = false, columnDefinition = BOOLEAN_DEFAULT_FALSE)
private boolean deleteYn;

@Column(name = "deleted_at")
private LocalDateTime deletedAt;
Copy link
Collaborator

Choose a reason for hiding this comment

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

제 개인적 견해로는 deletedAt 하나만으로 엔티티 삭제 여부를 알 수 있기에 deleteYndeletedAt은 중복이라고 생각합니다. 삭제된 시간과 삭제 여부를 따로 관리할 필요가 있는 케이스가 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

삭제된 엔티티를 복구할 경우, 이전에 삭제된 이력을 기록하는 것도 하나의 좋은 컨벤션이 될 수 있겠군요. 좋은 코드 제시 감사합니다!

Copy link
Author

Choose a reason for hiding this comment

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

좋은 의견 감사합니다. 제 개인적으로는 deletedAt에 deleteYn이 추가로 필요하다기보다, 오히려 deletedAt이 필수가 아니고 deletedYn으로 조회를 하는 게 좋다는 생각입니다.
언제 삭제되었는지에 대한 기록이므로 삭제 시간이 중요한 컬럼이라고 생각될 수 있지만, 대부분의 서비스 애플리케이션은 요청 및 응답 데이터를 MongoDB나 Elasticsearch같은 NoSQL이든 다른 곳이든 RDBMS가 아닌 곳에 따로 로깅을 합니다. 책임 소재 등의 문제가 발생하면 DB가 아닌 API 요청 로그를 봅니다. 따라서 deleted_at 컬럼의 필요성이 생각보다 높지 않고, modified_at 정도로도 충분하다는 생각입니다. 말씀하신 대로 삭제된 이력을 RDBMS에 기록할 때에도 모든 이력을 추적하기 위해 내부 컬럼보다는 별도의 히스토리 테이블을 사용하는 것이 일반적입니다. 따라서 효율과 직관성이 좋은 deleteYn만 사용하는 게 더 낫지 않나 하는 생각이 듭니다. 서비스가 커지면 DB에서 직접 튜플을 삭제할 일도 많아지기 때문에 직관적이지 않은 방식일수록 실수할 확률도 높아집니다. null은 값이 정해지지 않은 상태를 의미하므로, 복구 시 일부러 null을 삽입하는 것도 부자연스러운 작업이라고 생각합니다.

) {

public static MemberProfileUpdateResponseDto of(MemberProfile memberProfile, String profileImageUploadUrl) {
public static MemberProfileUpdateResponseDto from(MemberProfile memberProfile) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

fromof의 의미의 차이를 검색해 보았습니다.
of는 팩토리 메소드가 파라미터의 값을 변환하지 않고 매핑만 수행하고, from은 팩토리 메소드가 파라미터를 변환하는 작업을 거치는 것으로 이해했는데요,
이 메소드는 어떤 의미에서 이름을 of에서 from으로 변경하셨는지 여쭤봐도 괜찮을까요?

Copy link
Author

Choose a reason for hiding this comment

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

from은 말 그대로 ~로부터 만든다는 느낌이고, of는 넣은 대로 만든다는 느낌입니다. 즉, from이 보다 캡슐화에 충실한 메서드입니다. MemberProfileUpdateResponseDto가 어떤 상태값을 가지고 있는지는 모르지만 MemberProfile을 넣으면 새 객체를 만들어 준다는 것만 알면 충분할 때 from을 씁니다.
of는 호출 객체가 대상의 상태값을 알고 있어, of(name, age, email)같은 방식으로 생성할 값을 직접 넣어 줍니다. 이미 잘 알고 계시겠지만 그럴 거면 그냥 생성자를 호출해도 되나 결합도가 너무 높으므로 클래스 내부에서만 유지보수하기 위해 중간 단계를 둡니다. 예를 들어 name 필드가 사라지더라도 외부의 수십 수백 개의 of 메서드 호출은 그대로 두고, of 메서드 내부에서 생성자 호출 시 name만 제외하는 식으로 수정이 가능합니다.
하지만 여전히 다른 객체가 MemberProfileUpdateResponseDto 객체의 내부 구성에 대해 너무 많은 것을 알고 있어 상당한 결합도를 가지므로, 저는 of보다 from을 선호합니다.

어떤 글을 보셨는지 모르겠지만 찾아 보니 from과 of에 대해 간단하게 정리된 블로그 글이 있어 링크 걸어 두겠습니다.
https://sy-develop-note.tistory.com/74

return new MemberInterest(memberProfile, name);
}

// 삭제 사유: 사용되지 않아 삭제했으며, 명칭만 다를 뿐 setter의 역할을 하는 메서드이므로 바람직하지 않다고 보입니다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 구현 과정에서 불필요한 메소드를 삭제하는 걸 빠뜨렸었군요.
만약 회원 이름만 업데이트하는 경우가 필요하다고 가정한다면, updateName 메소드를 사용하는 게 맞을까요? 아니면 다른 더 좋은 방법이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

개인적으로는 불변 객체 또는 포장 객체를 선호합니다.

  1. 불변 객체는 name 필드만 업데이트하지 않고, 새로운 객체를 생성해서 다른 기존 값들과 변경된 name을 함께 넣어 새 엔티티를 생성 후 저장합니다. 기존의 영속성 컨텍스트가 끊어졌기 때문에 created_at과 modified_at을 수동으로 저장해 줘야 한다는 단점이 있습니다.
  2. 포장 객체는 엔티티 내부의 name 필드를 Name 객체로 포장합니다. 마찬가지로 수정할 때마다 update를 하는 것이 아니라 새로운 Name 객체를 생성해야만 하고, 그때마다 스스로 유효성 검사를 합니다. 따라서 누가 수정하든 DB에 저장되는 데이터의 무결성을 반드시 보장한다는 장점이 있습니다. 또한 자체로 Object 클래스의 equals와 hashcode 메서드를 오버라이딩하여, 소유 엔티티에 의존하지 않고 스스로 동등성을 비교합니다.

아래는 제가 예전에 구현했던 User 클래스인데, 내부 속성들이 포장되어 있습니다. 예시로 첨부하겠습니다.
https://github.com/zerobase-first-one/green-jangteo-backend/blob/main/src/main/java/com/firstone/greenjangteo/user/model/entity/User.java

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