Feature/member entity #32 #34

HiSeungmin · 2025-01-03T08:00:31Z

close: #32

바로 MERGE하지 마세요! 검토 후 MERGE해야합니다!

✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)

🏆 구현 목표

회원 관련 엔티티 생성 메서드 추가

📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)

회원 관련 엔티티 생성, 수정, 삭제 메서드 추가
Cascade 설정 (부모 엔티티 제거 시, 자식 엔티티 제거)
Orphan 설정 (고아 객체 제거)

- 회원 관련 엔티티 생성, 수정, 삭제 메서드 추가 - Cascade 설정 (부모 엔티티 제거 시, 자식 엔티티 제거) - Orphan 설정 (고아 객체 제거) close: #32

…nto feature/member-entity-#32

github-actions

Code review by ChatGPT

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Company.java

+    public void removeDepartment(Department department) {
+        this.departments.remove(department);
+        department.setCompany(null);
+    }
 }


코드 패치에 대한 간단한 코드 리뷰를 제공하겠습니다. 다음은 주의해야 할 버그 위험 및 개선 사항입니다:

Null Check: addDepartment 및 removeDepartment 메서드에서 전달된 department가 null인지 확인하는 로직이 없습니다. null 값이 전달될 경우 NullPointerException이 발생할 수 있으므로, 해당 메서드들에서 null 체크를 추가하는 것이 좋습니다.

불변성: departments 필드에 대해 불변 목록을 사용하는 것이 좋습니다. 외부에서 departments 리스트를 직접 수정할 수 없도록 getter 메서드에서 Collections.unmodifiableList()를 사용하여 반환하면 좋습니다.

메서드 일관성: createCompany 메서드와 updateCompany 메서드 사이에 생성 시 필드 설정에서 일관성이 부족할 수 있습니다. 예를 들어, businessType이나 businessRegistrationNumber 같은 필드는 업데이트 메서드에서 변경할 수 있도록 추가하는 것이 좋습니다.

JavaDoc 또는 주석 추가: 메서드에 대한 주석 또는 JavaDoc을 추가하여 각 메서드가 어떤 용도로 사용되는지를 문서화할 수 있습니다. 이는 코드 가독성을 높이고 유지 보수를 용이하게 합니다.

CascadeType.ALL: cascade = CascadeType.ALL를 사용할 경우, 모든 연관 엔티티가 해당 엔티티의 라이프 사이클에 영향을 받습니다. 종속 관계를 명확히 이해하고 있다면 괜찮지만, 불필요한 삭제나 업데이트가 발생할 수 있으니 주의해야 합니다.

부서 삭제 시 연관관계 관리: removeDepartment 메서드에서 부서를 삭제할 때, 해당 부서가 다른 회사와 연결되지 않도록 setCompany(null)을 호출하고 있지만, 이 작업이 논리적으로 필요할지에 대한 검토가 필요합니다. 부서가 다른 곳에서 참조될 가능성에 대한 예외 처리가 필요할 수 있습니다.

이러한 점들을 고려하고 코드를 수정하면, 전반적으로 더 안전하고 가독성이 좋은 클래스를 만들 수 있을 것입니다.

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Department.java

+    // 삭제 메서드
+    public void deleteDepartment() {
+        this.isActive = YN.N;
+    }
 }


코드 패치에 대한 간단한 리뷰를 진행하겠습니다.

버그 위험

위치 삭제 로직: removePosition(Position position) 메서드에서 전달된 position이 positions 리스트에 존재하지 않을 경우, remove 메서드가 false를 반환하지만, 이에 대한 처리가 필요 없으므로 문제가 발생하지는 않지만, 사용자에게 피드백을 제공하지 않습니다. 필요 시 로그를 추가하거나 예외를 던지는 것을 고려할 수 있습니다.

NullPointerException 위험: setDepartment(null)을 호출할 때, position이 null일 경우 NPE가 발생할 수 있으므로, 호출 전에 null 체크를 하는 것이 좋습니다.

단일 책임 원칙: deleteDepartment() 메서드는 부서를 활성 상태에서 비활성 상태로 변경하는 로직만을 수행합니다. 그러나 이 메서드 이름이 "삭제"라는 인상을 줄 수 있습니다. 실제로 데이터를 삭제하는 것이 아닙니다. 따라서 이름을 deactivateDepartment()와 같이 변경하는 것이 좋습니다.

개선 사항

정적 팩토리 메서드 명확성: createDepartment 메서드는 특정 로직이 없으므로, 더욱 명확하게 설명할 수 있는 추가 주석을 달면 좋습니다. 예를 들어, 각 매개변수가 왜 필요한지를 설명해주면 유지보수에 도움이 될 수 있습니다.

직책 관리 메서드의 트랜잭션 관리: addPosition 및 removePosition 메서드에서의 위치 추가 및 제거가 부서와 직책 간의 양방향 연관 관계를 관리하고 있습니다. 이 부분에 대해 더 명시적인 트랜잭션 처리가 필요할 수 있습니다. 예를 들어, JPA의 영속성 컨텍스트와 관련하여 상태가 변경될 때 명확히 이해될 수 있도록 추가 주석이 필요할 수 있습니다.

Immutable 리스트 사용 고려: positions 리스트가 ArrayList로 초기화되고, 외부에서 수정을 차단하려는 의도로 보입니다. 하지만, 외부에서 positions를 직접 변경할 수 없도록 getter를 제공할 때는 unmodifiableList를 이용하는 것도 고려할 수 있습니다.

총평

전반적으로 코드가 잘 구조화되어 있으며, JPA의 관계 매핑에 대한 이해가 잘 표현되어 있습니다. 위에서 언급한 몇 가지 개선 사항과 주의할 점들을 반영하면 더 견고한 코드가 될 것입니다.

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Member.java

        this.company      = company;
        this.password     = password;
        this.phoneNumber  = phoneNumber;
        this.departmentId = departmentId;
        this.positionId   = positionId;
+        this.profileImageExists = profileImageExists;
    }

    // 삭제 메서드


코드 패치에 대한 간략한 코드 리뷰를 진행하겠습니다.

수정 사항

프로필 이미지 존재 여부 제거: createMember 메서드에서 profileImageExists 파라미터가 제거되어, 멤버 생성 시 해당 속성을 초기화할 수 없게 되었습니다.

업데이트 메서드에 프로필 이미지 존재 여부 추가: updateMember 메서드에서 profileImageExists가 추가되었습니다.

잠재적인 문제

프로필 이미지 존재 여부: createMember 메서드에서 profileImageExists 파라미터를 제거함으로써 새로운 멤버를 생성할 때 프로필 이미지의 존재 여부를 설정하는 방법이 사라졌습니다. 만약 이 속성이 필수라면 생성을 하면서도 이를 관리할 수 있는 부분이 필요합니다. 사용자가 기대하는 대로 객체의 상태가 계속 일관되게 유지되지 않을 수 있습니다.

Null 값 처리: updateMember 메서드에서 profileImageExists에 대해 null 값을 전달할 경우, 이 값이 올바르게 처리되지 않을 위험이 있습니다. 이를 방지하기 위한 적절한 null 검사가 필요합니다.

개선 제안

생성 메서드에서 프로필 이미지 처리: createMember 메서드에 profileImageExists를 다시 추가하여 새로운 멤버 생성 시에도 프로필 상태를 설정할 수 있도록 하는 것이 좋습니다.

public static Member createMember(String loginId, String password, Company companyId, Role role, YN profileImageExists, String name, String email, LocalDate birthDate, String phoneNumber, Long departmentId, Long positionId) { // ... member.profileImageExists = profileImageExists; // 복원 return member; }

Validation 체크 추가: 두 메서드에서 입력받는 값들 (특히 읍차)의 유효성을 검증하는 로직이 추가되면 좋습니다. 예를 들어, 이메일 형식, 비밀번호 강도 등을 검증할 수 있습니다.

주석 추가: 코드의 의도가 명확하게 드러나지 않는 부분에 대해 주석을 추가하면 다른 개발자들이 이해하기 쉬워집니다.

이런 점들을 고려한다면 코드의 품질과 안정성을 향상시킬 수 있을 것입니다.

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberPasswordResetHistory.java

+        history.changeDate = LocalDateTime.now();
+        return history;
+    }
+
 }


코드 리뷰를 수행한 결과, 다음과 같은 개선 사항과 잠재적인 버그 리스크를 발견했습니다.

접근 제어자:

memberId, password, changer, changeIp, changeDate와 같은 필드는 패키지-private 상태로 보입니다. 이 필드들이 클래스 외부에서 접근할 필요가 있다면, 적절한 접근 제어자를 고려해야 합니다(예: private + getter 메서드).

null 가능성 검증:

메서드 createPwdHistory의 입력 매개변수들(memberId, password, changer, changeIp)에 대해 null 체크를 수행하는 것이 좋습니다. null 값이 들어올 경우, NullPointerException이 발생할 수 있습니다. 이를 방지하기 위해 예외를 던지거나 기본값을 설정하는 것이 좋습니다.

if (memberId == null || password == null || changer == null || changeIp == null) { throw new IllegalArgumentException("모든 필드는 null이 될 수 없습니다."); }

로깅:

비밀번호 변경 이력과 같은 민감한 정보가 포함되어 있으므로, 나중에 디버깅을 위해 로깅을 추가하는 것도 좋습니다. 특히 생성된 MemberPasswordResetHistory 객체의 필드들을 기록하면 도움이 될 수 있습니다.

불변성:

이력 객체는 일반적으로 불변성을 유지하는 것이 좋습니다. 필드에 대한 setter 메서드를 제공하지 않거나, 생성자만을 통해 초기화할 수 있도록 설계하는 것이 좋습니다. 현재 코드에서는 직접 필드에 접근하여 값을 설정하는 방식이기 때문에, 외부에서 객체 상태를 변경할 가능성이 있습니다.

변경 날짜:

changeDate는 객체를 생성할 때 현재 시간을 설정하고 있지만, 테스트 또는 다른 요구 사항에 따라 수동으로 설정할 수 있는 옵션이 있으면 유연성이 높아질 수 있습니다.

Javadoc 및 주석:

메서드에 대한 설명이나 사용 예시를 Javadoc으로 추가하는 것이 좋습니다. 현재 메서드가 어떤 목적으로 사용되는지를 명확히 하면 문서화에 도움이 될 것입니다.

위의 사항들을 고려하여 코드를 개선하면, 더 안전하고 유지 보수하기 쉬운 코드가 될 것입니다. 추가적인 코드 리뷰가 필요하시면 언제든지 문의해 주세요!

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberProfileHistory.java

+        return history;
+    }
+
+
 }


코드 패치에 대한 간단한 리뷰를 진행하겠습니다.

코드 리뷰

Null 체크: createProfileHistory 메서드에서 profileImageExists의 null 체크를 하고 있습니다. 그러나 이와 유사하게 다른 필드도 null 체크를 고려하는 것이 좋습니다. 예를 들어, 이메일, 이름 등의 필드가 null일 경우 어떻게 처리할지를 명시하는 것이 좋습니다. 현재로서는 이러한 필드에 대한 유효성 검사가 없습니다.

인수 검증: 메서드에 전달되는 파라미터들이 유효한지 검증하는 로직이 없습니다. 예를 들어, memberId, companyId가 음수일 경우 처리할 수 있는 로직이 필요할 수 있습니다.

Builder 패턴의 사용: 만약 생성할 항목이 많다면, Builder 패턴을 사용할 것을 추천합니다. 이를 통해 코드가 더욱 읽기 쉬워지고, 생성자 인수의 순서를 기억할 필요가 없어집니다.

기계적 코드 스타일: 현재 메서드의 선언 부분과 로직 사이에 불필요한 빈 줄이 있습니다. 코드의 가독성을 위해 한 줄만 사용하는 것이 좋습니다.

일관성 있는 주석: 한국어 주석이 추가된 점은 좋으나, 코드 내 다른 부분에도 주석을 추가하여 다른 개발자들이 더 쉽게 이해할 수 있도록 하는 것이 좋습니다.

의미 있는 메서드 이름: createProfileHistory라는 메서드 이름은 명확하나, 더 구체적으로 createMemberProfileHistory와 같은 이름으로 명명하면 무엇을 생성하는지 더 명확하게 전달할 수 있습니다.

결론

위의 몇 가지 검토 사항을 반영한다면 코드의 안정성 및 유지보수성이 향상될 것입니다. 추가적인 테스트 케이스를 작성하여 생성된 객체의 유효성을 검증하는 것도 좋은 방법입니다.

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Position.java

+    public void deletePostion() {
+        this.isActive = YN.N;
+    }
+
 }


코드 리뷰를 진행하겠습니다.

이름 오류:

deletePostion() 메서드의 이름에 오타가 있습니다. deletePosition()으로 수정하는 것이 좋습니다. 오타가 있는 메서드 이름은 코드 가독성을 떨어뜨리고, 다른 개발자들이 혼동할 수 있습니다.

은닉성:

createPosition() 메서드에서 Position 객체의 상태를 설정하는 부분이 잘 되어 있지만, isActive 필드 또한 초기화할 수 있도록 포함시키는 것이 좋습니다. 기본값으로 YN.Y가 설정되지만, 만들 때 명시적으로 활성화 여부를 설정할 수 있게 하면 더 유연한 코드가 될 것입니다.

메서드 이름:

setDepartment()가 부서를 설정하는 메서드인 것은 명확하지만, 만약 부서와 함께 다른 처리가 필요하다면 (예: 관련된 다른 상태 업데이트), 그 부분도 고려해서 메서드의 역할을 명확히 하는 것이 좋습니다.

Javadoc 주석:

생성 메서드와 각 메서드에 대한 설명을 추가하는 Javadoc 주석이 있으면 좋습니다. 이는 유지보수에 도움이 됩니다.

Null 체크:

createPosition() 및 setDepartment() 메서드에서 department, positionName, description 매개변수에 대해 null 체크를 추가하는 것이 좋습니다. NULL 값이 들어왔을 때 예외를 처리하면 더 안전한 코드를 작성할 수 있습니다.

단위 테스트:

새로운 메서드에 대해 단위 테스트를 작성하는 것을 권장합니다. 생성 메서드와 삭제 메서드의 동작을 확인하는 테스트가 있으면 나중에 리팩토링 시 확신을 가질 수 있습니다.

이와 같은 개선점을 고려하여 코드의 품질을 높일 수 있습니다.

github-actions · 2025-01-03T08:01:52Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Term.java

+    // 삭제 메서드
+    public void delete() {
+
+    }
 }


코드 패치에 대한 리뷰를 아래와 같이 진행하겠습니다.

코드 리뷰

null 체크 및 기본값 설정:

isRequired의 값이 null일 경우 기본값을 YN.N으로 설정하는 부분은 좋습니다. 그러나 사용자가 null을 전달했을 때 YN.N으로 대체되는 것이 의도된 동작인지 확인이 필요합니다. 만약 null이 의미가 다르다면, 이 부분에서 예외를 발생시키는 것이 좋습니다.

delete 메서드:

delete 메서드는 현재 구현되지 않았습니다. 삭제 기능은 제대로 구현해야 하며, 삭제가 비즈니스 로직에 어떤 방식으로 작동해야 하는지 명확히 정의해야 합니다. 예를 들어, 삭제할 때 로깅을 추가하거나 특정 조건을 검사해야 할 수 있습니다.

접근 제어자:

createTerm 메서드와 delete 메서드에 대한 접근 제어자가 명시되어 있지 않습니다. 메서드의 목적에 따라 public, private, protected 등의 적절한 접근 제어자를 정해야 합니다. 예를 들어, createTerm은 public으로 두고, delete는 내부 로직에서만 쓰며 private로 두는 것이 좋을 수 있습니다.

데이터 주입 방식:

createTerm 메서드에서 객체를 생성하는 패턴은 유용합니다. 그러나 객체 생성 시 불변성을 보장하려면 생성자(Constructor)를 활용하여 필수 필드나 기본 필드를 한 번에 설정하는 것이 더 안전할 수 있습니다.

주석:

메서드와 관련된 주석을 추가하면 코드의 가독성을 높이고, 다른 개발자들이 해당 메서드의 목적과 사용 방법을 쉽게 이해할 수 있습니다.

결론

전체적으로는 좋은 방향으로 진행되고 있으며, 몇 가지 개선할 점과 확인해야 할 사항이 있습니다. delete 메서드의 구현을 포함하여, 각 메서드의 접근 제어자 및 null 처리를 신중히 다루는 것이 중요합니다. 추가적으로 메서드에 대한 주석을 통해 가독성을 높여주는 것을 제안드립니다.

github-actions · 2025-01-03T08:01:53Z

src/main/java/com/seveneleven/devlens/domain/member/entity/TermsAgreementHistory.java

+        termsAgreeHistory.status    = status;
+        return termsAgreeHistory;
+    }
+
 }


코드 패치에 대한 간단한 리뷰를 진행하겠습니다.

변경 사항

컬럼 이름 변경:

terms_title에서 term_title로 변경되었습니다. (적절한 변경)

member_email에서 member_id로 변경되었습니다. (더 명확한 이름으로 변경)

agreement_status에서 is_agree로 변경되었습니다. (불리언 상태를 나타내는 더 나은 네이밍)

생성 메서드 추가:

createTermsAgreementHistory 메서드가 추가되어 객체를 생성하는 방식이 개선되었습니다. (생성 패턴 사용의 장점)

버그 위험性

데이터 타입 불일치:

termTitle 필드의 데이터 타입이 Long으로 되어 있는데, 이는 title을 나타내기에는 적합하지 않습니다. 일반적으로 제목은 문자열(String)로 처리해야 하므로 데이터 타입을 점검해야 합니다.

불리언 필드 네이밍 일관성:

is_agree라는 이름은 불리언 질의를 명확히 전달하지만, agreementStatus와 혼동할 수 있습니다. 가능하면 일관된 네이밍을 고려해 보세요.

개선 제안

명확한 데이터 타입 사용:

termTitle을 String으로 변경하여 명확성을 높이세요.

Javadoc 추가:

생성 메서드에 대한 Javadoc 주석을 추가하여 메서드의 사용법을 명확히 하면 좋습니다.

예외 처리:

생성 메서드 내에서 각 필드에 대한 유효성 검사를 추가하여, 잘못된 값이 들어오는 것을 방지하는 로직을 구현하는 것도 좋은 방법입니다.

변수명 개선:

is_agree보다는 agreement 또는 isAgreed 형식으로 카멜 케이스 스타일을 사용하는 것이 더 일반적입니다.

결론

코드 개선이 잘 이루어진 것 같으나, 몇 가지 버그 위험과 개선점을 고려해야 합니다. 전체적으로 코드의 의도는 명확하고, 수정 사항들이 긍정적입니다.

K-KY · 2025-01-03T09:44:50Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberProfileHistory.java

+    public static MemberProfileHistory createProfileHistory( Long memberId, String memberEmail, Long companyId, Role role, String name, String email,
+                                        LocalDate birthDate, String phoneNumber, Boolean profileImageExists, String department, String position ) {


칼럼이 그대로 들어가는 경우 그냥 엔티티를 넣는건 어떤가요?

좋은 방법인 것 같습니다. 적용하겠습니다.

K-KY · 2025-01-03T09:46:22Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberProfileHistory.java

+        history.phoneNumber = phoneNumber;
+        history.department  = department;
+        history.position    = position;
+        history.profileImageExists = profileImageExists != null ? profileImageExists : false;


true, false를 판단하는건 밖에서 하는게 더 좋을것 같아요, 그리고 Y, N 으로 하기로 했는데...

null이 들어온 시점에서 다른 인자들의 신뢰성도 의심되는 상황입니다. 처음부터 Null이 안들어오게 하면 좋을 것 같아요.

수정하겠습니다.

K-KY · 2025-01-03T09:48:27Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Term.java

+        Term term = new Term();
+        term.title      = title;
+        term.content    = content;
+        term.isRequired = isRequired != null? isRequired : YN.N;


저희 이렇게 정렬 하기로 했었나요?
term은 왜 안하셨나요

정렬이 어디에 들어갔죠?

K-KY · 2025-01-03T09:50:02Z

src/main/java/com/seveneleven/devlens/domain/member/entity/TermsAgreementHistory.java

+    @Column(name = "term_title", nullable = false)
+    private Long termTitle; // 약관 항목


약관 항목인데 Long이네요! 2진수로 저장하나여?

String으로 바꾸겠습니다 ㅎㅎ

kdh10806 · 2025-01-03T12:12:22Z

규영님 피드백 적용시킨 후에 올려주시면 다시 보겠습니다.

- 약관 동의 이력 테이블 상태 컬럼 제거 - 약관 이력 테이블 상태 컬럼, 상태 변경 메서드 추가 close: #32

- 회원 프로필 이력 생성 메서드 매개변수 엔티티로 수정 close: #32

- 약관 엔티티 약관 항목 필드 반환타입 수정 close: #32

github-actions

Code review by ChatGPT

github-actions · 2025-01-05T05:45:26Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Term.java

@@ -24,5 +26,28 @@ public class Term extends BaseEntity {

    @Column(name = "is_required", nullable = false)
    @Enumerated(EnumType.STRING)
-    private YN isRequired; // 필수 여부
+    private YN isRequired = YN.N; // 필수 여부


코드 패치를 검토한 결과 몇 가지 문제점과 개선사항이 보입니다.

초기화 중복:

클래스 필드 isRequired에 기본값 YN.N을 설정했지만, createTerm 메소드 내에서 isRequired가 null일 때 기본값을 설정하는 로직이 제거되었습니다. 이로 인해 외부에서 createTerm 메소드를 호출할 때 null이 전달되면 isRequired는 null이 될 수 있습니다. 이로 인해 코드의 안정성이 저하될 수 있습니다.

개선 제안:

term.isRequired = isRequired != null ? isRequired : YN.N;

이 줄을 다시 도입하는 것이 좋습니다. 또는 createTerm 메소드의 인자에서 기본값을 처리하는 방법을 고려할 수 있습니다.

코드 주석:

코드 내에 한글 주석이 사용되고 있습니다. 팀 내에 국제적인 개발자가 포함되어 있다면, 주석을 영어로 작성하는 것이 다국적 팀원들에게 더 유용할 수 있습니다.

변수 네이밍:

expiration_date와 같은 변수 이름에 언더스코어(_)가 포함되어 있습니다. 자바의 변수명 컨벤션에 따르면 카멜 케이스(camelCase)를 사용하는 것이 일반적입니다. 이 경우 expirationDate로 변경하는 것이 좋습니다.

날짜 타입:

expiration_date의 타입이 DateTime으로 되어 있는데, 자바에서는 LocalDateTime 또는 java.util.Date를 사용하는 것이 일반적입니다. 특히 java.time 패키지의 클래스를 사용하는 것이 더 안전하고 유연합니다.

null 체크:

메소드의 파라미터로 전달되는 인자들에 대해 null 체크를 수행하는 것도 좋은 습관입니다. 예를 들어, title, content 등이 null인 경우의 처리를 위해 예외를 던지거나 기본값을 설정하는 로직을 추가할 수 있습니다.

위의 제안들을 반영하면 코드의 안정성과 가독성을 높이는 데 도움이 될 것입니다.

github-actions · 2025-01-05T05:45:26Z

src/main/java/com/seveneleven/devlens/domain/member/entity/TermsAgreementHistory.java

    @Enumerated(EnumType.STRING)
-    private YN agreementStatus; // 약관 동의 여부


코드 패치에 대한 간단한 리뷰를 제공하겠습니다.

변경 사항 요약

termTitle의 타입이 Long에서 String으로 변경되었습니다.

잠재적인 버그 리스크

타입 변경으로 인한 일관성 문제: termTitle의 데이터 타입이 Long에서 String으로 변경됨에 따라, 기존에 이를 참조하고 있던 코드나 데이터베이스와의 불일치가 발생할 수 있습니다. 데이터베이스에서 이 필드의 타입도 변경되어야 할 것입니다.

호출하는 부분 업데이트 필요: createTermsAgreementHistory 메서드의 파라미터 타입이 변경되었으므로, 이 메서드를 호출하는 모든 위치를 찾아서 수정해야 합니다. 잘못된 타입으로 메서드를 호출할 경우 컴파일 에러가 발생할 것입니다.

개선 제안

주석 업데이트: termTitle의 데이터 타입이 변경됨에 따라 주석도 업데이트하여 현재의 타입이 String임을 명확히 하여 개발자가 혼동하지 않도록 해야 합니다.

유효성 검사 추가: termTitle의 내용에 대한 유효성을 검증하는 로직을 추가하는 것이 좋습니다. 예를 들어, 빈 문자열이 들어오지 않도록 하는 검사를 고려해 볼 수 있습니다.

의미 있는 네이밍: is_agree와 같은 변수를 좀 더 의미 있는 네이밍으로 변경할 수 있습니다. 예를 들어, agreementStatus와 같은 이름을 사용하는 것이 가독성을 높일 수 있습니다.

단위 테스트 추가: 변경된 코드에 대해 단위 테스트를 추가하여, 새로운 타입이 올바르게 작동하는지 확인할 필요가 있습니다.

이러한 사항들을 고려하여 코드 품질을 높이는 방향으로 개선하시길 권장합니다.

kdh10806 · 2025-01-05T06:43:47Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Member.java

MySql에서 생년월일은 date_of_birth(dob)로 많이 쓰입니다. 다음에 구현하실때 한 번 고려해보세요

네, 알겠습니다.

kdh10806 · 2025-01-05T06:45:33Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberPasswordResetHistory.java

change_ip 뜻이 모호합니다.
변경된 ip가 저장될 컬럼인거 같은데 '변경된'이라는 의미가 나타나도록 변경해보시는건 어떨까요

'changed_ip'로 수정하겠습니다.

kdh10806 · 2025-01-05T06:48:24Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberProfileHistory.java

@@ -44,11 +45,31 @@ public class MemberProfileHistory extends BaseEntity {
    private String phoneNumber; // 전화번호

    @Column(name = "profile_image_exists", nullable = false)
-    private Boolean profileImageExists; // 프로필 이미지 유무


Enumerated 빠져도 상관 없을까요

앗 어노테이션이 빠졌네요. 추가하겠습니다.

- 비밀번호 변경 이력 테이블 컬럼명 수정 - 프로필 이력 테이블 enum 어노테이션 추가 close: #32

- Member, Member 프로필 이력 엔티티 생년월일 컬럼명 수정 close: #32

github-actions

Code review by ChatGPT

github-actions · 2025-01-05T07:02:05Z

src/main/java/com/seveneleven/devlens/domain/member/entity/Member.java

@@ -48,7 +48,7 @@ public class Member extends BaseEntity {
    @Column(name = "email", nullable = false, length = 100)
    private String email; // 이메일

-    @Column(name = "birth_date")
+    @Column(name = "date_of_birth")
    private LocalDate birthDate; // 생년월일

    @Column(name = "phone_number", length = 20)


코드 패치에 대해 간단히 리뷰해 드리겠습니다.

명명 변경: birth_date에서 date_of_birth로 변경한 것은 더 명확하고 이해하기 쉬운 네이밍입니다. 가독성이 향상되었습니다.

부재 가능성: birthDate 필드에 대해 널(nullable) 제약 조건이 없는 부분이 눈에 띕니다. 생년월일이 필수인지 판단하여 필요한 경우 @Column(nullable = false)와 같은 제약 조건을 추가하는 것이 좋습니다.

주석 개선: 주석은 한국어로 잘 작성되었습니다. 하지만, 각 필드의 주석을 문서화할 때 다른 개발자나 팀원들이 필드의 용도를 쉽게 이해할 수 있도록 좀 더 구체적으로 작성할 수 있습니다.

진행 상황 주석: 코드에서 변경 사항이나 의도를 설명하는 주석이 없으므로, 이를 기록해 두면 향후 다른 개발자들이 이 코드를 이해하는 데 도움이 됩니다.

유효성 검증: 생년월일의 유효성 검증(예: 과거 날짜 여부 등)을 처리하는 로직이 필요한 경우 도입할 수 있습니다.

위 사항들을 고려하여 개선해 보시기 바랍니다. 코드 품질을 높이는 데 도움이 될 것입니다.

github-actions · 2025-01-05T07:02:05Z

src/main/java/com/seveneleven/devlens/domain/member/entity/MemberProfileHistory.java

이 코드 패치는 변수 이름을 birth_date에서 date_of_birth로 변경하고 있습니다. 다음은 코드에 대한 간단한 리뷰와 개선 사항입니다.

변경사항의 명확성:

birth_date를 date_of_birth로 변경한 이유가 주석 등에서 명확히 설명되어 있지 않습니다. 이 변경이 코드의 가독성을 향상시키거나 일관성을 높이기 위한 것이라면, 해당 이유를 주석으로 덧붙이는 것을 고려하는 것이 좋습니다.

일관성:

클래스 내의 다른 변수 명명 규칙과 일관성을 유지하는 것이 중요합니다. 혹시 다른 날짜와 관련된 변수들이 있다면, 동일한 명명 규칙을 적용하는 것이 좋습니다.

이메일 및 전화번호 필드:

email 필드가 nullable=false로 설정되어 있는데, 이는 사용자 데이터가 항상 필요하다는 것을 나타냅니다. 이 점은 괜찮지만, 이후 로직에서 사용자가 반드시 이메일을 지정하도록 강제하도록 구현된 부분이 있는지 확인해야 합니다.

phone_number 필드의 길이가 20으로 설정되어 있는데, 일반적으로 전화번호는 이보다 짧거나 길 수 있습니다. 실제로 수용해야 하는 최대 길이를 고려하여 적절한 길이를 설정하는 것이 좋습니다.

유효성 검사:

이 클래스 내에서 데이터 유효성 검사(예: 이메일 형식 검증, 전화번호 형식 검증 등)가 이루어지고 있는지 확인해야 합니다. 이는 오류를 방지하고 데이터의 무결성을 유지하는 데 도움을 줄 것입니다.

주석 작성:

주석이 한국어로 작성되어 있으나, 모든 팀원이 한국어를 이해하는지 확인할 필요가 있습니다. 국제화된 팀이라면 영어 주석도 고려할 수 있습니다.

위의 사항들을 고려하여 코드를 다듬으면 품질을 높이고 버그의 위험을 줄일 수 있습니다.

HiSeungmin added 2 commits January 3, 2025 16:56

Feat: 회원 Entity 생성 메서드 추가

6f1584b

- 회원 관련 엔티티 생성, 수정, 삭제 메서드 추가 - Cascade 설정 (부모 엔티티 제거 시, 자식 엔티티 제거) - Orphan 설정 (고아 객체 제거) close: #32

Merge branch 'dev' of https://github.com/Kernel360/KDEV3_DevLens_BE i…

bac81e3

…nto feature/member-entity-#32

HiSeungmin added the ✨ Feature 기능 개발 label Jan 3, 2025

HiSeungmin self-assigned this Jan 3, 2025

HiSeungmin requested review from Eseas, taekminKwon, K-KY, kdh10806 and jeongns2611 as code owners January 3, 2025 08:00

HiSeungmin linked an issue Jan 3, 2025 that may be closed by this pull request

[Feat] 회원 관련 엔티티 생성메서드 #32

Closed

1 task

github-actions bot reviewed Jan 3, 2025

View reviewed changes

K-KY approved these changes Jan 3, 2025

View reviewed changes

HiSeungmin added 3 commits January 5, 2025 14:28

Chore: 약관 이력 테이블 상태 컬럼 추가

3f5d28b

- 약관 동의 이력 테이블 상태 컬럼 제거 - 약관 이력 테이블 상태 컬럼, 상태 변경 메서드 추가 close: #32

Chore: 회원 프로필 이력 수정

9aff96a

- 회원 프로필 이력 생성 메서드 매개변수 엔티티로 수정 close: #32

Chore: 약관 이력 엔티티 수정

b7a546d

- 약관 엔티티 약관 항목 필드 반환타입 수정 close: #32

github-actions bot reviewed Jan 5, 2025

View reviewed changes

kdh10806 reviewed Jan 5, 2025

View reviewed changes

HiSeungmin added 2 commits January 5, 2025 15:58

Chore: 이력 테이블 컬럼 수정

168c26d

- 비밀번호 변경 이력 테이블 컬럼명 수정 - 프로필 이력 테이블 enum 어노테이션 추가 close: #32

Chore: 회원 엔티티 컬럼명 수정

82f0f7d

- Member, Member 프로필 이력 엔티티 생년월일 컬럼명 수정 close: #32

github-actions bot reviewed Jan 5, 2025

View reviewed changes

kdh10806 approved these changes Jan 5, 2025

View reviewed changes

HiSeungmin merged commit ef80ded into dev Jan 5, 2025
3 checks passed

HiSeungmin deleted the feature/member-entity-#32 branch January 16, 2025 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/member entity #32 #34

Feature/member entity #32 #34

HiSeungmin commented Jan 3, 2025

github-actions bot left a comment

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

github-actions bot Jan 3, 2025

K-KY Jan 3, 2025

HiSeungmin Jan 5, 2025

K-KY Jan 3, 2025

K-KY Jan 3, 2025

HiSeungmin Jan 5, 2025

K-KY Jan 3, 2025

HiSeungmin Jan 5, 2025

K-KY Jan 3, 2025

HiSeungmin Jan 5, 2025

kdh10806 commented Jan 3, 2025

github-actions bot left a comment

github-actions bot Jan 5, 2025

github-actions bot Jan 5, 2025

kdh10806 Jan 5, 2025

HiSeungmin Jan 5, 2025

kdh10806 Jan 5, 2025

HiSeungmin Jan 5, 2025

kdh10806 Jan 5, 2025

HiSeungmin Jan 5, 2025

github-actions bot left a comment

github-actions bot Jan 5, 2025

github-actions bot Jan 5, 2025

		public static MemberProfileHistory createProfileHistory( Long memberId, String memberEmail, Long companyId, Role role, String name, String email,
		LocalDate birthDate, String phoneNumber, Boolean profileImageExists, String department, String position ) {

		@Column(name = "term_title", nullable = false)
		private Long termTitle; // 약관 항목

		@Enumerated(EnumType.STRING)
		private YN agreementStatus; // 약관 동의 여부

Feature/member entity #32 #34

Feature/member entity #32 #34

Conversation

HiSeungmin commented Jan 3, 2025

✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)

🏆 구현 목표

📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

버그 위험

개선 사항

총평

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

수정 사항

잠재적인 문제

개선 제안

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

코드 리뷰

결론

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

코드 리뷰

결론

github-actions bot Jan 3, 2025

Choose a reason for hiding this comment

변경 사항

버그 위험性

개선 제안

결론

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdh10806 commented Jan 3, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jan 5, 2025

Choose a reason for hiding this comment

github-actions bot Jan 5, 2025

Choose a reason for hiding this comment

변경 사항 요약

잠재적인 버그 리스크

개선 제안

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jan 5, 2025

Choose a reason for hiding this comment

github-actions bot Jan 5, 2025

Choose a reason for hiding this comment