배치서비스에 위경도 변환 로직 추가 #35
Conversation
| @@ -65,6 +65,9 @@ dependencies { | |||
| // json | |||
| implementation group: 'org.json', name: 'json', version: '20210307' | |||
|
|
|||
| // coordinate | |||
| implementation group: 'org.osgeo', name: 'proj4j', version: '0.1.0' | |||
|
|
|||
| // ConfigurationProperties | |||
| annotationProcessor 'org.springframework.boot:spring-boot-configuration-processor' | |||
|
|
|||
There was a problem hiding this comment.
해당 코드 패치를 보면, 최근 버전의 "org.json" 라이브러리와 "org.osgeo.proj4j" 라이브러리가 추가되었습니다.
또한 Spring Boot ConfigurationProperties 어노테이션 프로세서가 적용되어 있습니다.
코드 리뷰에서는 누락된 코드 파트가 있는 것으로 보이며, 선언된 종속성들은 해결해야할 버그나 위험이 없는 것으로 보입니다.
추가적인 개선 사항을 판단하기 위해서는 해당 코드가 사용되는 문맥에 대한 이해가 필요합니다.
| private BigDecimal x; | ||
|
|
||
| @Column(name = "y") | ||
| @Digits(integer = 3, fraction = 6) | ||
| private BigDecimal y; | ||
|
|
||
| @Column(name = "ref_no", length = 50) |
There was a problem hiding this comment.
이 코드에서 새로운 엔티티 클래스 Cafe가 정의되고 있습니다.
여기서 주목해야할 부분은 @AllArgsConstructor, @NoArgsConstructor, @Builder 어노테이션이 있는 생성자입니다. 이러한 어노테이션은 Lombok을 사용하여 자동 생성된 생성자입니다. 특히, @AllArgsConstructor으로 생성된 생성자는 AccessLevel.PROTECTED 로 공개성이 제한되어 있습니다.
또한, 이 코드에서는 @Digits 어노테이션을 사용하여 BigDecimal 타입 속성인 x, y가 두 자리 정수와 여섯 자리 숫자까지를 가지도록 제한하고 있습니다.
하지만 BaseEntity에 대해서는 알 수 없으므로 해당 클래스의 내용 및 가능한 개선점 등 추가적인 정보가 필요합니다.
| public class Franchise { | ||
| @AllArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class Franchise extends BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) |
There was a problem hiding this comment.
이 코드는 Java 프로그램의 일부입니다.
코드 리뷰를 시작해보겠습니다:
Franchise클래스는BaseEntity를 상속합니다.FranchiseDto와jakarta.persistence.*패키지를 import 합니다.- Lombok을 사용합니다.
@Entity주석은 클래스가 JPA 엔티티임을 나타냅니다.@Table(name = "franchise")주석은 테이블 이름을 명시합니다.@Getter주석은 모든 비공개 필드에 대해 getter 메서드를 생성한다는 것을 나타냅니다.@AllArgsConstructor(access = AccessLevel.PROTECTED)주석은 생성자를 만들어 인자로 받은 값을 초기화하는 것을 의미합니다.@NoArgsConstructor(access = AccessLevel.PROTECTED)주석은 인자 없는 생성자를 만들어 호출합니다.
버그나 개선이 필요한 부분은 찾아보이지 않습니다. 단, 클래스안에서 존재하는 변수나 함수명을 보고 이 변수의 역할과 이름이 적절한지에 대해서 따져볼 필요가 있습니다. 또한 이 코드가 다른 부분과 함께 동작할 때 문제가 없는지 자세하게 검토해야합니다.
| public class Menu { | ||
| @AllArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class Menu extends BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) |
There was a problem hiding this comment.
이 코드 패치는 "com.aluminium.acoe.app.cafe.entity" 패키지에 위치한 Menu 엔티티 클래스를 수정하고 있습니다.
현재 코드에서 Lombok 애노테이션을 사용하여 축소 및 생성자를 자동으로 생성하고 있습니다. BaseEntity 클래스를 상속 받도록 변경된 점도 확인할 수 있습니다.
추가적인 개선사항으로 @AllArgsConstructor 어노테이션 대신 @requiredargsconstructor를 사용하면 final로 선언된 필드만 포함하여 생성자를 생성해주기 때문에 유지보수 측면에서 더 용이합니다.
또한, @builder 어노테이션은 객체의 생성 예시를 보여주는 목적으로 사용되는데, 현재 코드에서는 객체를 생성하는 데 필수적인 요소들이 전부 생략되어 있어서 해당 어노테이션의 사용 의미가 없어 보입니다. 만약 이전으로 돌아갈 수 없다면, Builder 방식을 사용하기 위해 활용하면 됩니다.
최종적으로, 현재 코드에서는 Menu 클래스의 생성자들이 protected로 선언되어 있는 것으로 보입니다. 따라서 다른 패키지에서 해당 클래스를 상속받아서 객체를 동적으로 생성할 수는 없어하는 점이 리스크가 될 수도 있습니다. 만약 본인의 코드 특성상 외부 클래스에서 상속을 할 가능성이 높다면 public 생성자로 변경하는 것이 좋습니다.
|
|
||
| // franchise setting | ||
| String cafeNm = cafeDto.getCafeNm(); | ||
| Franchise matched = franchises.stream() | ||
| .filter(fran -> cafeNm.contains(fran.getFranchiseNm())) | ||
| .findFirst().orElseGet(null); | ||
| .findFirst().orElseGet(() -> null); | ||
| // 프랜차이즈인 경우 프랜차이즈 할인가격 세팅 | ||
| if(matched != null) cafeDto.setDiscountAmt(matched.getDiscountAmt()); | ||
|
|
There was a problem hiding this comment.
위 코드 파트의 주석과 설명입니다:
Line 4: CoordinateUtil 클래스가 import되었는데 외부 라이브러리인지, 내부 클래스인지 알 수 없습니다.
Line 43-45: 좌표를 변환해서 decimal 형식으로 저장합니다.
하지만, 좌표계 변환에 실패할 경우 대응하지 않았습니다.
Line 51: matched 변수가 null일 때, Supplier가 null을 리턴하는 동작은 필요 없습니다.
위 코드에 대한 개선 사항과 버그 리스크를 고려한 리뷰였습니다.
| ProjCoordinate targetCoord = new ProjCoordinate(); | ||
| return transform.transform(sourceCoord, targetCoord); | ||
| } | ||
| } |
There was a problem hiding this comment.
위 코드 패치는 좌표변환을 위한 유틸리티 클래스입니다.
코드 자체에 큰 문제점은 없으나, 몇 가지 개선 사항이 있습니다.
먼저, CoordinateReferenceSystem 객체를 생성할 때 CRSFactory의 createFromName() 메서드를 호출합니다. 이 방식은 문자열로 좌표계를 지정하므로, 만약 잘못된 이름이 입력되면 예외가 발생합니다. 그러므로 올바른 좌표계 이름과 함께 EPSG 코드를 사용하는 것이 안전합니다.
또한, ProjCoordinate와 같은 중간 객체 대신에 DirectPosition 인터페이스를 사용하는 편이 더 정확합니다.
마지막으로, 이 코드는 라이브러리 의존성을 가지므로, 프로젝트 설정 등에서 해당 라이브러리가 추가되어있는지 확인해주어야 합니다.
resolving #28