-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/member entity #32 #34
Changes from all commits
6f1584b
bac81e3
3f5d28b
9aff96a
b7a546d
168c26d
82f0f7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ | |
import jakarta.persistence.*; | ||
import lombok.*; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
@Entity | ||
@Getter | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
|
@@ -20,6 +23,9 @@ public class Department extends BaseEntity { | |
@JoinColumn(name = "company_id", nullable = false, referencedColumnName = "id") | ||
private Company company; | ||
|
||
@OneToMany(mappedBy = "department", cascade = CascadeType.ALL, orphanRemoval = true) | ||
private List<Position> positions = new ArrayList<>(); // 부서에 속한 직책 리스트 | ||
|
||
@Column(name = "department_name", nullable = false, length = 50) | ||
private String departmentName; | ||
|
||
|
@@ -29,4 +35,35 @@ public class Department extends BaseEntity { | |
@Column(name = "is_active", nullable = false) | ||
@Enumerated(EnumType.STRING) | ||
private YN isActive = YN.Y; | ||
|
||
// 생성 메서드 | ||
public static Department createDepartment(Company company, String departmentName, String description) { | ||
Department department = new Department(); | ||
department.company = company; | ||
department.description = description; | ||
department.departmentName = departmentName; | ||
return department; | ||
} | ||
|
||
// 회사 설정 메서드 | ||
public void setCompany(Company company) { | ||
this.company = company; | ||
} | ||
|
||
// 직책 추가 메서드 | ||
public void addPosition(Position position) { | ||
this.positions.add(position); | ||
position.setDepartment(this); | ||
} | ||
|
||
// 직책 삭제 메서드 | ||
public void removePosition(Position position) { | ||
this.positions.remove(position); | ||
position.setDepartment(null); | ||
} | ||
|
||
// 삭제 메서드 | ||
public void deleteDepartment() { | ||
this.isActive = YN.N; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 간단한 리뷰를 진행하겠습니다. 버그 위험
개선 사항
총평전반적으로 코드가 잘 구조화되어 있으며, JPA의 관계 매핑에 대한 이해가 잘 표현되어 있습니다. 위에서 언급한 몇 가지 개선 사항과 주의할 점들을 반영하면 더 견고한 코드가 될 것입니다. |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MySql에서 생년월일은 date_of_birth(dob)로 많이 쓰입니다. 다음에 구현하실때 한 번 고려해보세요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네, 알겠습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대해 간단히 리뷰해 드리겠습니다.
위 사항들을 고려하여 개선해 보시기 바랍니다. 코드 품질을 높이는 데 도움이 될 것입니다. |
||
|
@@ -62,9 +62,8 @@ public class Member extends BaseEntity { | |
|
||
|
||
// 생성 메서드 | ||
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) { | ||
public static Member createMember(String loginId, String password, Company companyId, Role role, String name, String email, | ||
LocalDate birthDate, String phoneNumber, Long departmentId, Long positionId) { | ||
Member member = new Member(); | ||
member.name = name; | ||
member.role = role; | ||
|
@@ -76,17 +75,17 @@ public static Member createMember(String loginId, String password, Company compa | |
member.phoneNumber = phoneNumber; | ||
member.positionId = positionId; | ||
member.departmentId = departmentId; | ||
member.profileImageExists = profileImageExists; | ||
return member; | ||
} | ||
|
||
// 업데이트 메서드 | ||
public void updateMember(String password, String phoneNumber, Company company, Long departmentId, Long positionId) { | ||
public void updateMember(String password, String phoneNumber, Company company, Long departmentId, Long positionId, YN profileImageExists) { | ||
this.company = company; | ||
this.password = password; | ||
this.phoneNumber = phoneNumber; | ||
this.departmentId = departmentId; | ||
this.positionId = positionId; | ||
this.profileImageExists = profileImageExists; | ||
} | ||
|
||
// 삭제 메서드 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 간략한 코드 리뷰를 진행하겠습니다. 수정 사항
잠재적인 문제
개선 제안
이런 점들을 고려한다면 코드의 품질과 안정성을 향상시킬 수 있을 것입니다. |
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change_ip 뜻이 모호합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'changed_ip'로 수정하겠습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,25 @@ public class MemberPasswordResetHistory extends BaseEntity { | |
@Column(name = "password", nullable = false, length = 100) | ||
private String password; // 비밀번호 | ||
|
||
@Column(name = "change_date", nullable = false) | ||
private LocalDateTime changeDate; // 변경일시 | ||
@Column(name = "changed_date", nullable = false) | ||
private LocalDateTime changedDate; // 변경일시 | ||
|
||
@Column(name = "changer", nullable = false, length = 100) | ||
private String changer; // 변경자 | ||
|
||
@Column(name = "change_ip", nullable = false, length = 50) | ||
private String changeIp; // 변경 IP | ||
@Column(name = "changed_ip", nullable = false, length = 50) | ||
private String changedIp; // 변경 IP | ||
|
||
|
||
// 생성 메서드 | ||
public static MemberPasswordResetHistory createPwdHistory( Long memberId, String password, String changer, String changedIp ) { | ||
MemberPasswordResetHistory history = new MemberPasswordResetHistory(); | ||
history.memberId = memberId; | ||
history.password = password; | ||
history.changer = changer; | ||
history.changedIp = changedIp; | ||
history.changedDate = LocalDateTime.now(); | ||
return history; | ||
} | ||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 리뷰를 수행한 결과, 다음과 같은 개선 사항과 잠재적인 버그 리스크를 발견했습니다.
위의 사항들을 고려하여 코드를 개선하면, 더 안전하고 유지 보수하기 쉬운 코드가 될 것입니다. 추가적인 코드 리뷰가 필요하시면 언제든지 문의해 주세요! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package com.seveneleven.devlens.domain.member.entity; | ||
|
||
import com.seveneleven.devlens.domain.member.constant.Role; | ||
import com.seveneleven.devlens.domain.member.constant.YN; | ||
import com.seveneleven.devlens.global.entity.BaseEntity; | ||
import jakarta.persistence.*; | ||
import lombok.*; | ||
|
@@ -37,18 +38,39 @@ public class MemberProfileHistory 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) | ||
private String phoneNumber; // 전화번호 | ||
|
||
@Column(name = "profile_image_exists", nullable = false) | ||
private Boolean profileImageExists; // 프로필 이미지 유무 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enumerated 빠져도 상관 없을까요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앗 어노테이션이 빠졌네요. 추가하겠습니다. |
||
@Enumerated(EnumType.STRING) | ||
private YN profileImageExists; // 프로필 이미지 유무 | ||
|
||
@Column(name = "department", length = 100) | ||
private String department; // 부서명 | ||
|
||
@Column(name = "position", length = 100) | ||
private String position; // 직책 | ||
|
||
|
||
|
||
public static MemberProfileHistory createProfileHistory(Member member, String department, String position) { | ||
MemberProfileHistory history = new MemberProfileHistory(); | ||
history.memberId = member.getId(); | ||
history.memberEmail = member.getEmail(); | ||
history.companyId = member.getCompany().getId(); | ||
history.role = member.getRole(); | ||
history.name = member.getName(); | ||
history.email = member.getEmail(); | ||
history.birthDate = member.getBirthDate(); | ||
history.phoneNumber = member.getPhoneNumber(); | ||
history.department = department; | ||
history.position = position; | ||
history.profileImageExists = member.getProfileImageExists(); | ||
return history; | ||
} | ||
|
||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 간단한 리뷰를 진행하겠습니다. 코드 리뷰
결론위의 몇 가지 검토 사항을 반영한다면 코드의 안정성 및 유지보수성이 향상될 것입니다. 추가적인 테스트 케이스를 작성하여 생성된 객체의 유효성을 검증하는 것도 좋은 방법입니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,4 +30,24 @@ public class Position extends BaseEntity { | |
@Enumerated(EnumType.STRING) | ||
private YN isActive = YN.Y; | ||
|
||
|
||
// 생성 메서드 | ||
public static Position createPosition( Department department, String positionName, String description) { | ||
Position position = new Position(); | ||
position.department = department; | ||
position.description = description; | ||
position.positionName = positionName; | ||
return position; | ||
} | ||
|
||
// 부서 설정 메서드 | ||
public void setDepartment(Department department) { | ||
this.department = department; | ||
} | ||
|
||
// 삭제 메서드 | ||
public void deletePostion() { | ||
this.isActive = YN.N; | ||
} | ||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 리뷰를 진행하겠습니다.
이와 같은 개선점을 고려하여 코드의 품질을 높일 수 있습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
package com.seveneleven.devlens.domain.member.entity; | ||
|
||
import com.seveneleven.devlens.domain.member.constant.TermsStatus; | ||
import com.seveneleven.devlens.domain.member.constant.YN; | ||
import com.seveneleven.devlens.global.entity.BaseEntity; | ||
import jakarta.persistence.*; | ||
import lombok.*; | ||
import org.joda.time.DateTime; | ||
|
||
@Entity | ||
@Getter | ||
|
@@ -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; // 필수 여부 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치를 검토한 결과 몇 가지 문제점과 개선사항이 보입니다.
위의 제안들을 반영하면 코드의 안정성과 가독성을 높이는 데 도움이 될 것입니다. |
||
|
||
@Column(name = "expiration_date") | ||
private DateTime expiration_date; // 만료 일자 | ||
|
||
@Column(name = "status", length = 50) | ||
@Enumerated(EnumType.STRING) | ||
private TermsStatus status; // 상태 | ||
|
||
|
||
// 생성 메서드 | ||
public static Term createTerm(String title, String content, YN isRequired, TermsStatus status) { | ||
Term term = new Term(); | ||
term.title = title; | ||
term.content = content; | ||
term.isRequired = isRequired; | ||
term.status = status; | ||
return term; | ||
} | ||
|
||
// 상태 변경 메서드 | ||
public void updateStatus(TermsStatus status) { | ||
this.status = status; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 리뷰를 아래와 같이 진행하겠습니다. 코드 리뷰
결론전체적으로는 좋은 방향으로 진행되고 있으며, 몇 가지 개선할 점과 확인해야 할 사항이 있습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,17 +20,25 @@ public class TermsAgreementHistory extends BaseEntity { | |
@Column(name = "term_id") | ||
private Long termId; // 약관 이력 ID | ||
|
||
@Column(name = "terms_title", nullable = false) | ||
private Long termsTitle; // 약관 항목 | ||
@Column(name = "term_title", nullable = false) | ||
private String termTitle; // 약관 항목 | ||
|
||
@Column(name = "member_email", nullable = false) | ||
private Long memberEmail; // 회원 ID | ||
@Column(name = "member_id", nullable = false) | ||
private Long memberId; // 회원 ID | ||
|
||
@Column(name = "agreement_status", nullable = false) | ||
@Column(name = "is_agree", nullable = false) | ||
@Enumerated(EnumType.STRING) | ||
private YN agreementStatus; // 약관 동의 여부 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 간단한 리뷰를 제공하겠습니다. 변경 사항 요약
잠재적인 버그 리스크
개선 제안
이러한 사항들을 고려하여 코드 품질을 높이는 방향으로 개선하시길 권장합니다. |
||
private YN is_agree; // 약관 동의 여부 | ||
|
||
|
||
// 생성 메서드 | ||
public static TermsAgreementHistory createTermsAgreementHistory( Long termId, String termTitle, Long memberId, YN is_agree) { | ||
TermsAgreementHistory termsAgreeHistory = new TermsAgreementHistory(); | ||
termsAgreeHistory.termId = termId; | ||
termsAgreeHistory.termTitle = termTitle; | ||
termsAgreeHistory.memberId = memberId; | ||
termsAgreeHistory.is_agree = is_agree; | ||
return termsAgreeHistory; | ||
} | ||
|
||
@Column(name = "status", length = 50) | ||
@Enumerated(EnumType.STRING) | ||
private TermsStatus status; // 상태 | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드 패치에 대한 간단한 리뷰를 진행하겠습니다. 변경 사항
버그 위험性
개선 제안
결론코드 개선이 잘 이루어진 것 같으나, 몇 가지 버그 위험과 개선점을 고려해야 합니다. 전체적으로 코드의 의도는 명확하고, 수정 사항들이 긍정적입니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 코드 리뷰를 제공하겠습니다. 다음은 주의해야 할 버그 위험 및 개선 사항입니다:
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)
을 호출하고 있지만, 이 작업이 논리적으로 필요할지에 대한 검토가 필요합니다. 부서가 다른 곳에서 참조될 가능성에 대한 예외 처리가 필요할 수 있습니다.이러한 점들을 고려하고 코드를 수정하면, 전반적으로 더 안전하고 가독성이 좋은 클래스를 만들 수 있을 것입니다.