Conversation
|
안녕하세요 열심히 임해주신 것에 대해서 저도 좋은 자극을 받았음에 감사하면 코드 리뷰 시작하겠습니다. |
| public class MovieController { | ||
| private final MovieService movieService; | ||
|
|
||
| public MovieController(MovieService movieService) { |
There was a problem hiding this comment.
root 프로젝트에서 lombok 디펜던시를 추가했던데 굳이 @requiredargsconstructor를 사용하지 않고 생성자로 만들어서 사용한 이유가 있을까요? 어떤 것을 사용하든 상관은 없지만 개인적으로는 가독성적인 측면에서 @requiredargsconstructor 를 사용하는 것이 좀 더 깔끔하다고 생각이 들어요!
멀티 모듈에서 해등 모듈의 롬복이 없는 것도 아니라서요!
| this.movieService = movieService; | ||
| } | ||
|
|
||
| @GetMapping("/bookable") |
There was a problem hiding this comment.
URL의 자원을 명사로 표기하는 것을 권장하고 있습니다.
예약 가능 여부는 query parameter로 구분: /api/movies?bookable=true
이런 식으로 표기하는 것이 좀 더 좋지 않을까? 싶습니다. 이후 확장성 측면에서도 유리할 것 같아 보입니다.
|
|
||
| @CreatedDate | ||
| @Column(name = "reg_dttm", nullable = false, updatable = false) | ||
| private LocalDateTime regDttm; |
There was a problem hiding this comment.
컨벤션을 어떻게 설계하느냐에 따라 다르지만 개인적으로는 축약어를 사용하는 것을 지양하는 것이 좋을것 같아요. 새로운 팀원이 들어오거나 기존의 인원이 아닌 사람이 코드를 수정하거나 작업할 때 이런 축약어 제한을 풀어두면 코드가 점점 가독성이 떨어질 가능성이 있기 때문에 개인적으로는 선호 하지는 않아요.
| @Column(name = "reg_dttm", nullable = false, updatable = false) | ||
| private LocalDateTime regDttm; | ||
|
|
||
| @Column(name = "chg_user_id") |
There was a problem hiding this comment.
JPA 컨벤션은 카멜 케이스로 작성된 코드를 스네이크 코드로 변경 시켜주는 것이 디폴트 설정으로 잡혀 있어요. 그래서 굳이 불필요한 코드를 작성할 필요가 없습니다!
또한 위에서도 언급했지만 제가 처음 봤을 때 chg 라는 축약어가 뭔지 몰라서 불필요하게 구조를 뜯어보고 어떤 의미인지 유추하게 되었는데 이런 요소들은 생산성을 저하 시킬 원인이 되기도 합니다! 한번 고려해주시면 좋을 것 같아요!
| private Long seatId; | ||
|
|
||
| @Column(name = "seat_type_cd") | ||
| private String seatTypeCd; |
ddl.sql
Outdated
| `CHG_USER_ID` VARCHAR(20) NULL COMMENT '수정자', | ||
| `CHG_DTTM` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '수정일', | ||
| PRIMARY KEY (`BOX_ID`) | ||
| DROP TABLE IF EXISTS `box`; |
There was a problem hiding this comment.
네이밍을 좀 더 명확한 의미의 단어를 사용하는 것이 더 좋을 것 같아요. Cinema, Theater 같이 말이죠! box 라는 단어를 극장이라고 사용하기는 하는데 일반적으로 사용하지는 않아서 헷갈리수도 있을 것 같아요.
ddl.sql
Outdated
| `CHG_USER_ID` VARCHAR(20) NULL COMMENT '수정자', | ||
| `CHG_DTTM` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '수정일', | ||
| PRIMARY KEY (`TIME_ID`) | ||
| DROP TABLE IF EXISTS `timetable`; |
There was a problem hiding this comment.
스케쥴이라는 단어가 더 맞지 않을까? 혹시 스케쥴이 예약어라 사용하지 못한것이라면 movie_scheule 이라고 작성하는 것이 좀 더 의미가 명확하게 전달 될 것 같아요.
좋았던 점
아쉬웠던 점
총평이카텍처가 잘 구성되어 었는지는 현재 구조로는 알기가 쉽지 않을 것 같아요. 현재 작성된 코드가 api/movie/bookable 만 있어서 다른 케이도 작성되어야 소영님이 의도한 것이 어떤 것인지 확인이 가능할 것 같습니다. DB 구조를 보니까 어떤 곳은 date, time 이런 것을 사용하고 있고 다른 곳에서는 dttm 같은 축약어도 사용하고 있어요. 해당 컬럼의 구조는 통일해주는 것이 좋을 것 같아요. |
3 participants
작업 내용
테이블 구조
테이블 : 영화, 장르, 상영관, 상영시간표, 좌석, 회원, 예매
아키텍처(Hexagonal Architecture)
cinema-core=> 도메인, 엔티티, 비즈니스 로직
cinema-application=> 유스케이스(Use Case)
cinema-adapter=> (Inbound) REST Controller, CLI
=> (Outbound) JPA Repository, External API
cinema-infra=> DB 연결, 외부 라이브러리 설정
cinema-common=> 공통으로 사용할 수 있는 유틸리티, 상수, 예외 처리 클래스
발생했던 문제와 해결 과정을 남겨 주세요.
*************************** APPLICATION FAILED TO START *************************** Description: Failed to configure a DataSource: 'url' attribute is not specified and no embedded datasource could be configured. Reason: Failed to determine a suitable driver class Action: Consider the following: If you want an embedded database (H2, HSQL or Derby), please put it on the classpath. If you have database settings to be loaded from a particular profile you may need to activate it (no profiles are currently active).-> compose.yaml, application.yml 파일을 수정해보았으나 해결하지 못하였음.
Docker compose실행은 정상적으로 되는 상태이나 app url 연결에 문제가 발생하는 것으로 보임.이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
기타 질문