Skip to content
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.

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

Week1 step3 #724

Open
wants to merge 18 commits into
base: sanghou
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Add sectionCreate. (ask lazy injection)
Sanghou committed Feb 5, 2023
commit b7147a17b3738fc0a1ea03b9e06e3a2961d318aa
50 changes: 43 additions & 7 deletions src/main/java/subway/line/Line.java
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@

import static javax.persistence.FetchType.LAZY;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import javax.persistence.Entity;
@@ -45,6 +46,28 @@ public Line(String name, String color, Station upStation, Station downStation,
this.color = color;
this.upStation = upStation;
this.downStation = downStation;
this.sections = new ArrayList<>();
this.distance = distance;
}

public Line(Long id, String name, String color, Station upStation, Station downStation, Long distance) {
this.id = id;
this.name = name;
this.color = color;
this.upStation = upStation;
this.downStation = downStation;
this.sections = new ArrayList<>();
this.distance = distance;
}

public Line(Long id, String name, String color, Station upStation, Station downStation,
List<Section> sections, Long distance) {
this.id = id;
this.name = name;
this.color = color;
this.upStation = upStation;
this.downStation = downStation;
this.sections = sections;
this.distance = distance;
}

@@ -82,13 +105,26 @@ public Line updateLine(String name, String color) {
return this;
}

public void addSection(Section section) {
if (!Objects.equals(downStation, section.getUpStation())) {
throw new AddNonDownStationToSectionException();
public void addSection(Section add) {
System.out.println("line domain add Section");
if (sections.size() != 0 && !Objects.equals(downStation.getId(), add.getUpStation().getId())) {
Sanghou marked this conversation as resolved.
Show resolved Hide resolved
System.out.println("error1");
throw new InvalidUpstationAppendInSection();
}
if (sections.contains(section)) {
throw new DuplicatedStationAddToSectionFailException();

System.out.println(sections.size());
if (sections.size() != 0) {
System.out.println(sections.get(0).getUpStation());
System.out.println(sections.get(0).getDownStation());
System.out.println(add.getUpStation());
}
if (sections.stream().anyMatch(s -> Objects.equals(add.getDownStation().getId(), s.getUpStation().getId()))) {
Copy link

Choose a reason for hiding this comment

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

  • 하나의 분기문에서 수행하기에는 꽤 긴 작업으로 보이는데 메서드로 분리하는게 어떨까요?
  • add라는 변수명이 구간정보를 표현할 수 있을까요?
  • 객체에서 연산에 필요한 값을 메서드체이닝으로 꺼내고 있네요 디미터법칙에 어긋나지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

메서드 체이닝으로인한 디미터의 법칙을 지키지 못하는 문제는
Station에 public class로 equals를 정의해서 equals의 대상으로 id만 쓰고 있다는 정보도 숨겼는데 이런 접근 방식을 시도해보는 것을 말씀해주신 것이 맞을까요?
혹시 다른 방법들이 추가로 있을까요?

Objects.equals(add.getDownStation().getId(), s.getUpStation().getId())
=>
s.getUpStation().equals(add.getDownStation())

System.out.println("error2");
throw new DuplicatedStationAddToSectionFailException();
};

sections.add(add);
downStation = add.getDownStation();
}

@ResponseStatus(value = HttpStatus.BAD_REQUEST)
@@ -99,8 +135,8 @@ public DuplicatedStationAddToSectionFailException() {
}

@ResponseStatus(value = HttpStatus.BAD_REQUEST)
static class AddNonDownStationToSectionException extends RuntimeException {
public AddNonDownStationToSectionException() {
static class InvalidUpstationAppendInSection extends RuntimeException {
public InvalidUpstationAppendInSection() {
super("하행역에만 새로운 구간 추가가 가능합니다.");
}
}
9 changes: 7 additions & 2 deletions src/main/java/subway/line/LineResponse.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package subway.line;

import java.util.List;
import subway.section.Section;
import subway.station.Station;

public class LineResponse {
@@ -9,14 +11,16 @@ public class LineResponse {
private String color;
private Station upStation;
private Station downStation;
private List<Section> sections;
private Long distance;

public LineResponse(Long id, String name, String color, Station upStation, Station downStation, Long distance) {
public LineResponse(Long id, String name, String color, Station upStation, Station downStation, List<Section> sections, Long distance) {
this.id = id;
this.name = name;
this.color = color;
this.upStation = upStation;
this.downStation = downStation;
this.sections = sections;
this.distance = distance;
}

@@ -27,12 +31,13 @@ public static LineResponse of(Line line) {
line.getColor(),
line.getUpStation(),
line.getDownStation(),
line.getSections(),
line.getDistance()
);
}

public Line toEntity() {
return new Line(name, color, upStation, downStation, distance);
return new Line(id, name, color, upStation, downStation, sections, distance);
}

public Long getId() {
12 changes: 11 additions & 1 deletion src/main/java/subway/line/LineService.java
Original file line number Diff line number Diff line change
@@ -5,6 +5,8 @@
import java.util.stream.Collectors;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import subway.section.SectionCreateRequest;
import subway.section.SectionService;
import subway.station.Station;
import subway.station.StationService;

@@ -13,20 +15,28 @@ public class LineService {

private final LineRepository lineRepository;
private final StationService stationService;
private final SectionService sectionService;

public LineService(LineRepository repository, StationService stationService) {
public LineService(LineRepository repository, StationService stationService, SectionService sectionService) {
this.lineRepository = repository;
this.stationService = stationService;
this.sectionService = sectionService;
}

@Transactional
public LineResponse saveLine(LineCreateRequest request) {
Station upStation = stationService.findById(request.getUpStationId()).toEntity();
Station downStation = stationService.findById(request.getDownStationId()).toEntity();

Line created = lineRepository.save(
new Line(request.getName(), request.getColor(), upStation, downStation, request.getDistance())
);

sectionService.createSection(created.getId(), new SectionCreateRequest(upStation.getId(), downStation.getId(), request.getDistance()));

System.out.println("Line service");
System.out.println(created.getSections().size());

return LineResponse.of(created);
}

5 changes: 3 additions & 2 deletions src/main/java/subway/section/SectionController.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package subway.section;

import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PathVariable;
@@ -17,8 +18,8 @@ public SectionController(SectionService sectionService) {
}

@PostMapping("/lines/{id}/sections")
public void createSection(@PathVariable Long id, @RequestBody SectionCreateRequest request) {
sectionService.createSection(id, request);
public ResponseEntity<SectionResponse> createSection(@PathVariable Long id, @RequestBody SectionCreateRequest request) {
return ResponseEntity.ok().body(sectionService.createSection(id, request));
}

@DeleteMapping("/lines/{lineId}/sections")
Copy link

Choose a reason for hiding this comment

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

같은 Resource(ex: id)인데 어디선 id 어디선 lineId라고 이름을 다르게 지을 필요가 있을까요?!

11 changes: 8 additions & 3 deletions src/main/java/subway/section/SectionService.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package subway.section;

import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import subway.line.Line;
@@ -10,11 +11,11 @@
@Service
public class SectionService {
Copy link

Choose a reason for hiding this comment

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

엄밀히 말해 구간 서비스는 필요 없는 서비스 계층입니다. 왜일까요?

구간정보는 결국 노선정보에 종속되며 같은 라이프사이클을 가져가는 엔티티라는게 힌트입니다.

Copy link
Author

Choose a reason for hiding this comment

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

아하! 애초에 순환 참조가 발생한 이유도 같이 동작해야하는 로직이었기 때문이겠네요!

Copy link
Author

Choose a reason for hiding this comment

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

이렇게 된다면 LineService에서 Section 저장을 위해서 SectionRepository를 참조하게 될텐데
괜찮은 방법일까요?
구간이라는 개념이 노선에 종속적이기 때문에 구간 조회에 대한 책임도 노선 서비스가 담당하는게 맞을까요?


private final SectionRepository sectionRepository;
private final StationService stationService;
private final LineService lineService;
private final SectionRepository sectionRepository;

public SectionService(StationService stationService, LineService lineService, SectionRepository sectionRepository) {
public SectionService(StationService stationService, @Lazy LineService lineService, SectionRepository sectionRepository) {
this.stationService = stationService;
this.lineService = lineService;
this.sectionRepository = sectionRepository;
@@ -23,12 +24,16 @@ public SectionService(StationService stationService, LineService lineService, Se
@Transactional
public SectionResponse createSection(Long lineId, SectionCreateRequest request) {
Station upStation = stationService.findById(request.getUpStationId()).toEntity();
Station downStation = stationService.findById(request.getUpStationId()).toEntity();
Station downStation = stationService.findById(request.getDownStationId()).toEntity();
Line line = lineService.showLine(lineId).get().toEntity();

Section created = sectionRepository.save(new Section(upStation, downStation, request.getDistance()));

line.addSection(created);

System.out.println("sectionService addSection");
System.out.println(line.getSections().size());

return SectionResponse.of(created);
}

5 changes: 5 additions & 0 deletions src/main/java/subway/station/Station.java
Original file line number Diff line number Diff line change
@@ -34,4 +34,9 @@ public Long getId() {
public String getName() {
return name;
}

@Override
public String toString() {
return "id : " + id + ", name : " + name;
}
}
2 changes: 2 additions & 0 deletions src/main/java/subway/station/StationService.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package subway.station;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@@ -9,6 +10,7 @@
@Service
@Transactional(readOnly = true)
public class StationService {

private final StationRepository stationRepository;

public StationService(StationRepository stationRepository) {
2 changes: 1 addition & 1 deletion src/test/java/subway/section/SectionAcceptanceTest.java
Original file line number Diff line number Diff line change
@@ -33,9 +33,9 @@ public class SectionAcceptanceTest {
@DisplayName("지하철 구간 생성 테스트")
void 노선_구간_생성_테스트() {
// given
Long 신규역아이디 = StationTestUtils.지하철역_생성(MockStation.신림역);
Line line = LineTestUtils.역_과_노선_생성(LineCreateRequestDTO.서울2호선_노선_생성요청);
Long 기존_노선_하행역_ID = line.getDownStation().getId();
Long 신규역아이디 = StationTestUtils.지하철역_생성(MockStation.신림역);

//when
SectionCreateRequest request = new SectionCreateRequest(