[1단계 - 지하철 정보 관리 기능] 허브(방대의) 미션 제출합니다. #16
Conversation
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
Co-authored-by: miseongk <miseongkinn@gmail.com>
| Line persistLine = findLineById(id); | ||
| return LineResponse.of(persistLine); | ||
| public void delete(final Long id) { | ||
| lineRepository.findById(id) |
There was a problem hiding this comment.
delete 의 경우에는 id 가 존재하던 안하던 lineRepository.deleteById(id); 를 수행할 수 있지 않을까요?
요걸 에러상황이라고 생각하지 않아도 되지 않을까해서요! 허브생각은 어떠신가요?
delete는 멱등성을 갖는 행위이니 id가 있던 없던 해당 요청에의한 서버의 상태는 항상 동일해야하지 않을까 해서요!
There was a problem hiding this comment.
저는 lineRepository.deleteById(id); 수행 여부가 멱등성을 지키는지의 여부를 결정한다고 생각하지 않습니다!
처음에는 204 다음 요청에는 리소스가 존재하지 않으니 404 를 반환하여 클라이언트에게 더 많은 정보를 줄 뿐, 리소스가 제거된 서버의 상태는 멱등성을 지키고 있다고 생각했습니다.
RFC 7231에는 해당 메서드를 사용하는 여러 개의 동일한 요청이 서버에 미치는 영향이 단일 요청에 대한 효과와 동일한 것을 멱등성이라고 설명하고 있고 mdn에서는 DELETE의 상태 코드는 응답마다 달라질 수 있지만, 그럼에도 멱등성을 가진다고 설명하고 있습니다.
사실 원론적인 이야기일 수 있어서 정해진 API 규칙에 따라서 200을 계속 내려주는 것도 좋다고 생각하는데, 서브웨이는 어떻게 생각하시나요?
| return saveLine(line, newLineEntity); | ||
| } | ||
| final LineEntity updateLineEntity = lineEntity.orElseThrow(LineNotFoundException::new); | ||
| sectionDao.deleteAll(updateLineEntity.getId()); |
There was a problem hiding this comment.
효율을 떠나서 운영리소스를 이렇게 전체를 이렇게 쉽게 다 삭제해버리도 괜찮을까요?
만약에 코드가 복잡해져서, 실수로 delete만 되버리면 어떻게 되나요? 이를 바로 대응할 무엇인가가 필요할거같은데, 그런게 아니라면 delete하는 형태는 피해야하지 않을까요
There was a problem hiding this comment.
전체 삭제한다면 문제가 발생할 경우 대응을 못할 것 같습니다 😢
라인의 값을 변경하는 경우, 전체 제거 + 전체 추가가 아닌 변경사항만 반영하는 방법으로 변경했습니다!
| @@ -145,6 +149,23 @@ public Station findStationByName(final String name) { | |||
| .orElseThrow(StationNotFoundException::new); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
아 요거 제가 말씀드린거는 아예 재정의를 할필요가 없는 것 같다 였는데요.
객체끼리 비교하는 니즈 또는 line 객체를 해쉬컬렉션의 키로 사용하는것이 있는게 아니라면, 그냥 제거하고 id만 사용하면 되지 같은지 않나 해서요!
Line a = new Line(..) // id 1
Line b = new Line(..) // id 2
- a.id == b.id 직접 비교 가능
- Map<Long, Line> ... id를 키로 들고 있음
이렇게 하면 재정의할 필요가 없을 것 같아서요!
There was a problem hiding this comment.
슬프게도 현재 Repository에서 hashSet을 사용하고 있는데, id equals가 없으면 로직이 너무 길어지더라고요 😢
외부 요소를 위해 추가된 거긴 하지만, 식별자가 있으면 식별자로 비교하는게 맥락상 맞는 것 같아 넣어놨습니다!
| lineDao.update(new LineEntity(line.getId(), line.getName(), line.getColor())); | ||
| addNewStation(line); | ||
| addNewSection(line); | ||
| deleteRemovedSection(line, savedLine); |
There was a problem hiding this comment.
허브 요거 Section, station이 지금도 더하고 삭제하도록 로직이 되어있는데요. 혹시
- line update
- station update
- section udpate
이렇게 업데이트만 못하나요?
만약에 디비에 id가 없이 station과 section이 저장되어있으면 이렇게 하는게 맞는데, 지금 id가 존재를 해서 추적이 가능하자나요? 그래서 update가 가능하지 않을까 라고 생각했습니다. 한번 가능한지 여부 확인해주시고 가능하다면 다음단계때 적용해주세요!
만약에 로직으로 풀어내기 어렵다면 그냥 지금처럼 해주세요~
There was a problem hiding this comment.
현재 요구사항에 따라 라인이 변경되는 경우를 정리해보자면
- 라인에 초기 역들이(2개의 역, 1개의 구간) 추가된다.
- 라인에 구간을 제거한다.
- 2개의 역 밖에 없을 때 -> 2개의 역, 1개의 구간 제거
- 1개의 역을 제거하는 경우 -> 1개의 역, 1개의 구간 제거
- 라인에 역을 추가한다. (1개의 역, 1개의 구간)
따라서 insert 또는 delete로 이루어집니다.
구간의 내용을 수정하거나, 역의 내용을 수정하는 부분이 존재하지 않아서, update를 사용하지 않았습니다.
내용 추가
비즈니스 로직에서 역 사이에 역을 추가할 때 구간을 새로 생성하는 방법으로 구현했었는데 해당 부분을 수정한다면 update를 사용할 수 있을 것 같습니다 👍
안녕하세요 서브웨이~
전 허브🌿라고 합니다!
TDD 사이클을 지켜가며 Domain -> Repository, Dao -> Service -> Controller 순으로 코드를 작성했습니다.
시간 내에 마치기 위해서 어쩔 수 없이 적절히 트레이드오프를 한 부분이 있었습니다.
빠르게 구현하려고 Line에 역을 추가할 때 전체 제거하고 전체 추가하는 방향으로 작성을 했는데요!
따라서 라인의 리소스 위치가 변경되는 문제가 현재 애플리케이션에 존재합니다.
또한 시간이 부족하여 Controller를 포함한 통합테스트를 할 때 예외에 대한 부분을 작성하지 못했습니다.
지하철 미션동안 잘 부탁드립니다 😄
5월 12일 내용 추가