Skip to content

Refactor/put project #296

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.

Sign up for GitHub

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

Merged
Eseas merged 2 commits into from
Feb 19, 2025
Merged

Refactor/put project #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b6e0b3
refactor : project modify company id can't modify
Eseas Feb 19, 2025
294f27e
refactor : project modify company id can't modify
Eseas Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Admin/src/main/java/com/seveneleven/project/dto/PutProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public static class Request {
private Long projectTypeId; // 프로젝트 유형 ID
private ProjectStatusCode projectStatusCode; //
private String bnsManager; // BNS 담당자 ID (Member 엔티티의 ID)
private String contractNumber; // 계약서 번호
private LocalDate plannedStartDate; // 시작 예정일
private LocalDate plannedEndDate; // 종료 예정일
private LocalDate startDate; // 시작일
Expand All @@ -54,7 +53,6 @@ public Project updateProject(
projectName,
projectDescription,
projectStatusCode,
contractNumber,
plannedStartDate,
plannedEndDate,
startDate,
Expand All @@ -76,7 +74,6 @@ public static class Response {
private String projectType; // 프로젝트 유형 ID
private ProjectStatusCode projectStatusCode; //
private String bnsManager; // BNS 담당자 ID (Member 엔티티의 ID)
private String contractNumber; // 계약서 번호
private LocalDate plannedStartDate; // 시작 예정일
private LocalDate plannedEndDate; // 종료 예정일
private LocalDate startDate; // 시작일
Expand All @@ -101,7 +98,6 @@ private Response(Project project, List<ProjectTag> tags, List<ProjectAuthorizati
projectDescription = project.getProjectDescription();
projectType = project.getProjectType().getProjectTypeName();
bnsManager = project.getBnsManager();
contractNumber = project.getContractNumber();
plannedStartDate = project.getPlannedStartDate();
plannedEndDate = project.getPlannedEndDate();
startDate = project.getStartDate(); // 시작일
Copy link

@github-actions github-actions bot Feb 19, 2025

Choose a reason for hiding this comment

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

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

  1. 속성 제거: contractNumber 필드가 Request와 Response 클래스에서 제거되었습니다. 이 변경이 불필요한 경우(예를 들어, 계약 번호가 여전히 필요하다면), 누락으로 인해 데이터 누락이나 링크 문제가 발생할 수 있습니다. 이 부분에서 요구사항 변경이나 기존의 기능이 여전히 필요한지 다시 확인해야 합니다.

  2. 데이터 무결성: 만약 contractNumber를 제거하면서 비즈니스 로직이나 데이터베이스에서 관련 제약 조건이 있다면, 그런 제약이 이제 불필요해졌는지 확인해야 합니다. 관련 데이터가 제대로 처리되지 않을 위험이 있습니다.

  3. 주석 처리: 클래스 속성에 대한 주석이 잘 작성되어 있습니다. 다만, 더 복잡한 비즈니스 로직이 있는 경우, 각 변수의 활용도 및 변화되는 맥락에 대해 추가적인 주석을 남기는 것이 좋습니다.

  4. 자료형 및 포맷: LocalDate는 잘 사용되고 있으나, 날짜와 관련된 다른 포맷이 자주 필요할 수 있습니다. 가능한 경우, 날짜 관련 함수를 추가하여 유연성을 높일 수 있습니다.

  5. 에러 핸들링: 프로젝트 업데이트 시 발생할 수 있는 오류를 적절히 처리하는지 확인해야 합니다. 예를 들어, null 체크나 데이터 유효성 검사를 통해 사용자에게 친숙한 에러 메시지를 제공하는 것이 중요합니다.

종합적으로, contractNumber 제거에 따른 영향 및 데이터 무결성 검토가 필요하고, 코드의 전반적인 안정성을 높이기 위해 추가적인 에러 핸들링이나 주석 강화를 고려해야 합니다.

Expand Down
2 changes: 0 additions & 2 deletions Common/src/main/java/com/seveneleven/entity/project/Project.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ public Project update(
String name,
String description,
ProjectStatusCode statusCode,
String contractNumber,
LocalDate plannedStartDate,
LocalDate plannedEndDate,
LocalDate startDate,
Expand All @@ -132,7 +131,6 @@ public Project update(
this.projectType = ProjectType;
this.projectStatusCode = statusCode;
this.bnsManager = bnsManager;
this.contractNumber = contractNumber;
this.plannedStartDate = plannedStartDate;
this.plannedEndDate = plannedEndDate;
this.startDate = startDate;
Copy link

@github-actions github-actions bot Feb 19, 2025

Choose a reason for hiding this comment

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

코드 리뷰 결과는 다음과 같습니다:

  1. 버그 위험: contractNumber 필드가 제거되었지만, 이 필드가 사용되던 다른 부분에서도 함께 수정이 이루어졌는지 확인해야 합니다. 만약 이 필드를 의도적으로 삭제한 것이라면, 프로젝트와 관련된 다른 클래스나 메소드에서 contractNumber를 사용하는 부분이 없는지 점검이 필요합니다.

  2. 정보의 일관성: contractNumber가 제거되면서 해당 데이터와 관련된 검증 로직이나 비즈니스 로직도 함께 점검되어야 합니다. 만약 이 데이터가 중요하다면, 적절한 방식으로 데이터를 처리할 필요가 있습니다.

  3. 주석 또는 문서화: 변경 이유나 의도를 명확하게 주석으로 남겨두는 것이 좋습니다. 다른 개발자들이 코드를 이해하는 데 도움이 될 것입니다.

  4. 유닛 테스트: contractNumber가 삭제됨에 따라 관련된 유닛 테스트가 있는지 확인하고, 해당 테스트를 업데이트하거나 제거해야 할 것입니다.

  5. 가독성 향상: 변수명이 충분히 의미를 나타내지 않을 경우, ProjectType과 같은 변수명에 대해 더 명확한 이름을 사용하는 것이 좋습니다. 가독성을 높이기 위해 가능한 한 모든 변수명을 일관되게 사용하는 것이 중요합니다.

이 외에도, 전체적인 코드 구조와 다른 메소드나 클래스와의 관계를 고려하여 추가적인 피드백을 받을 수 있을 것입니다.

Expand Down
Loading