[스티치] 지하철 정보 관리 - 인수테스트 미션 제출합니다 #6
Conversation
- AdminLine.js 코드 리팩토링 - AdminLine에서 사용될 Modal인 LineModal.js 구현
| @Column("updated_at") | ||
| private LocalDateTime updatedAt; | ||
|
|
||
| private Set<LineStation> stations = new LinkedHashSet<>(); |
There was a problem hiding this comment.
오, LinkedHashSet을 사용하셨군요. 지금도 문제가 없지만... LinkedHashSet은 유니크성만 부여하고 있어서 유용한 메서드들을 제공해 주는 관점에서 LinkedList가 좀 더 효율적으로 사용될 것 같네요. 예를 들어, 엘리먼트의 before, after를 불러 올 수도 있고 첫번째인지 마지막인지 여부 등등
There was a problem hiding this comment.
LinkedList로 구현을 변경하였습니다.
그런데 확장성 고려를 위해 stations의 타입을 List으로 두는 것이 일반적이라고 알 고 있습니다.
그러면 LinkedList의 메서드들을 사용할 수 없던데 어떻게 해결할 수 있을까요?
There was a problem hiding this comment.
아래와 같이 직접 타입을 선언하면 됩니다ㅎ
LinkedList< LineStation > stations = new LinkedList<>()
| this.updatedAt = LocalDateTime.now(); | ||
| } | ||
|
|
||
| public void addLineStation(LineStation lineStation) { |
There was a problem hiding this comment.
LinkedList의 경우 exist()로 존재 여부만 체크하고 push()하면 끝인듯 합니다ㅎㅎ
There was a problem hiding this comment.
이번 미션에서 LineStation을 추가할 때 preStationId와 stationId를 확인하면서 순서에 맞게 추가해야 하더라구요.
그래서 해당 로직은 최대한 다시 정리하되 구조는 그대로 가져가는 식으로 구현했는데 혹시 제가 하비님의 말을 잘못 이해한 걸까요?
| stations.add(lineStation); | ||
| } | ||
|
|
||
| private boolean isHeadStation(LineStation lineStation) { |
There was a problem hiding this comment.
LinkedList로 처리하면 indexOf() or getFist()로 체크 가능
| return Objects.isNull(lineStation.getPreStationId()); | ||
| } | ||
|
|
||
| private LineStation findHeadLineStation() { |
There was a problem hiding this comment.
이 부분도 for문을 타게 되면 n만큼 돌아야 되는데 getFirst()같은 메서드를 사용하면 될 것 같습니다.
There was a problem hiding this comment.
해당 부분을 구조를 변경하면서 더 효율적이도록 수정하였습니다!!
| } | ||
|
|
||
| public LineResponse findLineWithStationsById(Long id) { | ||
| final Line persistLine = lineRepository.findById(id) |
There was a problem hiding this comment.
persistLine이라는 변수명이 이상하네요. 실제로 영속된 객체가 아닌것 같습니다.
영속되기 위해서는 선행 조건이 필요합니다ㅎㅎ
There was a problem hiding this comment.
findById()로 Line 객체를 불러오기 보다는 id만 뽑아오는게 성능에 좋습니다.
There was a problem hiding this comment.
변수 명은 수정하였습니다.
그리고 해당 메서드에서 실제로 사용하는 값은 line 객체의 List 인데 이는 연관 객체? 참조 객체인데 실제 line 테이블에는 저장되는 값이 아니니까 Line 객체를 불러와야 하는 것 아닌가요?
| public LineResponse findLineWithStationsById(Long id) { | ||
| final Line persistLine = lineRepository.findById(id) | ||
| .orElseThrow(RuntimeException::new); | ||
| List<Station> stations = persistLine.getLineStationsId() |
There was a problem hiding this comment.
for문을 돌며 n번씩 station 테이블을 탐색할것 같네요. stationId를 이어 붙여서 where in절로 한번만 호출 가능합니다.
네트워크 레이턴시가 성능에 영향을 주는 부분이기 때문에 이런 사소한 부분도 잡아주면 좋을 것 같네요ㅎ
|
|
||
| public void save(Long id, LineStation lineStation) { | ||
| final Line persistLine = lineRepository.findById(id).orElseThrow(RuntimeException::new); | ||
| persistLine.addLineStation(lineStation); |
There was a problem hiding this comment.
위에서도 영속에 대해 말하였지만 선행조건이 필요합니다. 영속성을 사용할 수 있도록 변경해 주세요.
There was a problem hiding this comment.
아래의 CUD(Create, Update, Delete)들도 영속성을 사용하도록 변경해 주세요.
|
|
||
| @DisplayName("지하철 노선을 관리한다") | ||
| @Test | ||
| void manageLine() { |
There was a problem hiding this comment.
when then에 대하여 더 가독성이 좋게끔 변경이 되었으면 좋겠습니다. 케이스를 파악하는데 코드보다는 주석이 더 빠르다고 생각해서요:)
There was a problem hiding this comment.
이번에 처음 ATDD를 해보면서 제공된 주석인데 아직 ATDD의 시나리오를 어떻게 작성하는 것이 좋은지 잘 모르겠습니다...
혹시 해당 부분에 대해서 어떤 식으로 수정하는 것이 좋을까요?
There was a problem hiding this comment.
지금처럼 기존의 역이 만들어지고 새로운 역이 추가 되었을 때, 중복되었을 때, 순서가 변경되었을 때, 문제가 있는 값일 때 등의 테스트가 만들어지면 좋을 것 같습니다.
|
피드백 주신 내용들에 대해 적용하였습니다. 추가적으로 영속성을 위한 @transactional 어노테이션, 유효성 테스트를 어노테이션으로 하기 위한 방법들을 찾아보고 적용해보면서 계속 리팩토링중에 있습니다. 우선 말씀하신 부분들에 대해 피드백 적용하여서 미션 다시 제출하겠습니다!!! |
|
다른 미션을 진행하면서 시간이 부족하여 이제야 다시 제출하는 점 정말 죄송합니다 ㅜㅡㅜ
등을 기준으로 리팩토링을 진행하였습니다. |
* feat: 노선 등록/목록/삭제 기능 구현 * feat: 노선 조회 기능 구현 * feat: 노선 수정 기능 구현 * feat: 중복된 노선/역 이름에 대한 예외처리 * fix: 중복된 이름에 대한 상태 코드 변경 * test: 테스트 메소드 실행 전에 Dao를 초기화하는 기능 추가 * test: 노선 기능에 대한 E2E 테스트 * refactor: jdbc 기반으로 전환 * refactor: jdbc 기반으로 전환 * refactor: jdbc 기반으로 전환 * refactor: 코드 구조 리팩토링 * Merge pull request #1 from woong7/step2 * feat: 구간 관련 로직 일부 추가 * Merge pull request #2 from woong7/step2 * fix: 테스트 오류 수정 * Merge pull request #3 from woong7/step2 * refactor: 코드 전체 리팩토링 * Merge pull request #4 from woong7/step2 * refactor: Lombok / Transaction / Bean Validation 적용 * refactor: 서비스 레이어 경량화 * refactor: 전체 리팩토링 * refactor: Sections 도메인 추가 * refactor: Lines / Stations 도메인 추가 * Merge pull request #5 from woong7/step2 * feat: 역 정렬 로직 추가 * fix: 재귀 오류 수정 * refactor: 정렬 로직 수정 * Merge pull request #6 from woong7/step2 * refactor: 피드백 반영 * Merge pull request #7 from woong7/step2 * test: Sections 테스트코드 추가 * refactor: Stations / Lines 일급 컬렉션 제거 * fix: 테스트 오류 수정 * Merge pull request #8 from woong7/step2
인수테스트 미션 제출합니다.
프론트랑 웹을 처음 해봐서 부족한 점이 많습니다.
리뷰해주셔서 감사합니다 :)