[Spring Core] 안준영 미션 제출합니다. #306

Junyeong-An · 2024-07-11T20:18:35Z

구현

계층별 클라이언트와 통신하는 controller, 비즈니스 로직을 처리하는 service, DB에 액세스하는 dao로 나누었습니다.
계층간 데이터 이동을 위해 dto를 사용하여 도메인의 훼손을 막았습니다.
기존 controller에서 처리되는 dao 접근 로직을 service로 옮겨 계층간의 책임을 더 명확히 했습니다.

고민할 점

서비스 계층에서 다른 서비스를 의존성 주입받아도 되는 지...?
계층을 어떻게 더 자세히 나눠야 할 지...
예외 처리 부분을 어떻게 깔끔하게 할 수 있을 지 고민 중입니다.

@DisplayName

* 1단계 - 홈화면 구성 * 2단계 - 예약 조회 * 3단계 - 예약 추가/취소 * 4단계 - 예외 처리 * 4단계 - 예외 처리 Reservation추가 * 테스트 모두 통과하도록 수정 * 불필요 공백 제거 * Test 코드에 @DisplayName추가 * thymeleaf 의존성 추가 * Feat: 5단계: 데이터베이스 적용하기 * Feat: 6단계: 데이터 조회하기

…oomescape-playground into junyeong-an

YehyeokBang

미션 학습 중에 봤던 전역 예외에 대해 다시 학습하고 적용해보면 더 좋을 것 같아요!

고생하셨습니다!

YehyeokBang · 2024-07-12T12:40:54Z

src/main/java/roomescape/controller/RoomescapeController.java

+@Controller
+public class RoomescapeController {
+
+    private final RoomDAO RoomDAO;
+    private final ReservationService reservationService;
+
+    public RoomescapeController(RoomDAO roomDAO, TimeService timeService, ReservationService reservationService) {
+        this.RoomDAO = roomDAO;
+        this.reservationService = reservationService;
+    }
+
+    @GetMapping("/reservation")
+    public String reservation() {
+        return "new-reservation";
+    }
+
+    @GetMapping("/reservations")
+    @ResponseBody
+    public ResponseEntity<List<ReservationDto>> getAllReservations(){
+        List<ReservationDto> reservations = RoomDAO.findAll().stream()
+                .map(reservation -> new ReservationDto(reservation.getName(), reservation.getDate(), reservation.getTime()))
+                .collect(Collectors.toList());
+        return new ResponseEntity<>(reservations, HttpStatus.OK);
+    }


RoomDAO 변수명은 소문자로 시작하는 것이 관례입니다!

ReservationService를 따로 구현하신 것 같은데, RoomDAO`도 주입받아서 사용하시는 이유가 있나요?

생성자 매개변수에 TimeService 객체를 주입받도록 선언되어 있는데 이유가 무엇인가요?

컨트롤러에서는 클라이언트와의 소통을 하는 계층이르모 RoomDAO보다는 service의 의존성을 주입받는 게 맞는 것 같습니다...!

YehyeokBang · 2024-07-12T12:42:32Z

src/main/java/roomescape/controller/RoomescapeController.java

+    @PostMapping("/reservations")
+    public ResponseEntity<?> createReservation(@RequestBody ReservationDto reservationDto) {
+        try {
+            Reservation reservation = reservationService.addReservation(reservationDto);
+            URI location = UriComponentsBuilder.fromPath("/reservations/{id}")
+                    .buildAndExpand(reservation.getId())
+                    .toUri();
+            return ResponseEntity.created(location).body(reservation);
+        } catch (IllegalArgumentException e) {
+            return ResponseEntity.badRequest().body(e.getMessage());
+        }
+    }


createReservation() 메서드는 예약을 생성하는 요청과 그 응답에 집중하도록 하고 Location 필드를 위해 URI를 만드는 부분은 따로 메서드로 분리하거나 아래와 같이 응답 구조를 변경하면 가독성에 도움이 될 것 같아요!

return ResponseEntity.status(HttpStatus.CREATED) .header("Location 헤더 설정") .header("필요한 다른 헤더들~~") .body("본문 객체");

추가로 Controller에서 try-catch 블럭을 통해 예외를 잡아서 처리하는 것보다는 미션 학습 자료에 있는 @ControllerAdvice 어노테이션에 대해 학습하고 전역 예외 처리에 대해 고민해보면 좋을 것 같아요!

YehyeokBang · 2024-07-12T12:44:34Z

src/main/java/roomescape/dao/RoomDAO.java

+    public void delete(int id) {
+        jdbcTemplate.update("DELETE FROM reservation WHERE id = ?", id);
+    }


Suggested change

public void delete(int id) {

jdbcTemplate.update("DELETE FROM reservation WHERE id = ?", id);

}

public void deleteById(int id) {

jdbcTemplate.update("DELETE FROM reservation WHERE id = ?", id);

}

메서드명을 좀 더 명확히 작성하면 좋을 것 같아요!

YehyeokBang · 2024-07-12T12:45:18Z

src/main/java/roomescape/domain/Reservation.java

+    public void setId(int id) {
+        this.id = id;
+    }


setter를 사용하신 이유가 있나요?

필요없는 것 같아 삭제했습니다...😅

YehyeokBang · 2024-07-12T12:46:03Z

src/main/java/roomescape/domain/Time.java

+    int id;
+    String time;


private 접근 지정자를 사용하면 더 좋을 것 같아요! 그 이유에 대해 학습하고 준영이 직접 알려주세요!

클래스 외부에서 내부로 접근하지 못하게 막을 수 있어 정보은닉 및 객체는 자신의 데이터를 스스로 관리하기 때문에 캡슐화가 가능합니다.

YehyeokBang · 2024-07-12T12:50:10Z

src/main/java/roomescape/service/TimeService.java

+    public List<TimeResDto> getAllTimes() {
+        List<Time> times = timeDAO.findAll();
+        return times.stream()
+                .map(TimeResDto::from)
+                .collect(Collectors.toList());
+    }


Suggested change

public List<TimeResDto> getAllTimes() {

List<Time> times = timeDAO.findAll();

return times.stream()

.map(TimeResDto::from)

.collect(Collectors.toList());

}

public List<TimeResDto> getAllTimes() {

List<Time> times = timeDAO.findAll();

return times.stream()

.map(TimeResDto::from)

.toList();

}

저는 toList()가 UnmodifiableList 를 반환하기 때문에 주로 사용해요!

준영이 직접 collect(Collectors.toList())와 toList()에 대해서 학습하고 차이를 알려주세요!

Collectors.toList()는 가변 리스트를 반환하지만 toList()는 불변 리스트를 반환합니다. 컬렉션을 수정할 필요가 있는 경우 Collectors를 사용하고 불변성을 유지하고 싶을 경우 toList()를 사용한다고 합니다. 결과값을 불변성으로 보장하기 위해 toList()를 사용하는 것이 좋아보이는 것 같습니다 감사합니다! 😊

sangjin6439 · 2024-07-13T14:47:12Z

src/main/java/roomescape/controller/RoomescapeController.java

+    private final RoomDAO RoomDAO;
+    private final ReservationService reservationService;
+
+    public RoomescapeController(RoomDAO roomDAO, TimeService timeService, ReservationService reservationService) {


TimeService를 매개변수로 포함하신 이유가 있으신가요?

사용되지 않는데 잘못 포함한 것 같습니다... 😅

src/main/java/roomescape/controller/RoomescapeController.java

src/main/java/roomescape/controller/TimeController.java

sangjin6439 · 2024-07-13T14:53:44Z

src/main/java/roomescape/dao/TimeDAO.java

+                    time);
+        } catch (EmptyResultDataAccessException e) {
+            return null;
+        }


try-catch 문을 사용해도 좋지만 코드가 지저분해 보일 수 있습니다. 다른 방식의 예외처리를 생각해 보시거나 검증 로직을 추가해도 좋을 거 같습니다.

sangjin6439 · 2024-07-13T14:55:40Z

src/main/java/roomescape/dto/ReservationDto.java

+        return new ReservationDto(name, date, time);
+    }
+}
+


record와 정적팩토리 메서드를 사용하신 이유가 궁금합니다!

record를 사용하면 자동으로 생성자, 접근자 메서드를 생성해주어 불필요한 코드를 줄이고 가독성을 높일 수 있습니다. 간결하고 불변 객체를 만들기 위해 사용되므로 dto를 만드는데 사용하는 게 목적에 맞겠다 싶어 사용했습니다.

정적팩토리 메서드는 생성자와 달리 메서드에 이름을 부여할 수 있어 생성 의도를 명확히 전달할 수 있고 생성 로직을 캡슐화 하고 반환 타입을 유연하게 할 수 있다는 장점이 있어 사용했습니다.

dd-jiyun

고생하셨습니다 ❗️

dd-jiyun · 2024-07-15T01:16:44Z

src/main/java/roomescape/controller/RoomescapeController.java

+        return new ResponseEntity<>(reservations, HttpStatus.OK);
+    }
+
+    @PostMapping("/reservations")


@GetMapping과 @DeleteMapping에서는 @ResponseBody 어노테이션을 사용하셨는데 @PostMapping에만 사용하지 않은 이유가 있나요??

ResponseEntity를 반환하는 경우 http응답 본문으로 사용되는 것을 Spring이 처리해주므로 @ResponseBody 애노테이션이 필요하지 않는다고 합니다..!

dd-jiyun · 2024-07-15T01:19:17Z

src/main/java/roomescape/service/TimeService.java

+import java.util.stream.Collectors;
+
+@Service
+public class TimeService {


@Transactional에 대해서 학습하시고 알려주세요!

@Transactional은 데이터베이스 작업의 일관성과 신뢰성을 보장하는 어노태이션입니다. 적용하면 여러 데이터베이스 작업이 하나의 트랜잭션 내에서 실행되며, 트랜잭션 중 오류가 발생할 경우 모든 작업이 롤백된다고 합니다. 따라서 작업의 일관성과 신뢰성을 보장한다고 합니다. 서비스 코드에 한 번 적용시켜 보겠습니다. 감사합니다!

YehyeokBang

고생하셨습니다!

YehyeokBang · 2024-07-24T17:04:52Z

src/main/java/roomescape/controller/TimeController.java

+@RestController
+@RequestMapping("/times")
+public class TimeController {
+    private final TimeService timeService;
+
+    public TimeController(TimeService timeService) {
+        this.timeService = timeService;
+    }
+
+    @PostMapping
+    @ResponseBody
+    public ResponseEntity<TimeResDto> addTime(@RequestBody TimeDto timeDto) {
+        TimeResDto timeResDto = timeService.addTime(timeDto);
+        HttpHeaders headers = createHeader("/times/" + timeResDto.id());
+        return new ResponseEntity<>(timeResDto, headers, HttpStatus.CREATED);
+    }


Suggested change

@RestController

@RequestMapping("/times")

public class TimeController {

private final TimeService timeService;

public TimeController(TimeService timeService) {

this.timeService = timeService;

}

@PostMapping

@ResponseBody

public ResponseEntity<TimeResDto> addTime(@RequestBody TimeDto timeDto) {

TimeResDto timeResDto = timeService.addTime(timeDto);

HttpHeaders headers = createHeader("/times/" + timeResDto.id());

return new ResponseEntity<>(timeResDto, headers, HttpStatus.CREATED);

}

@RestController // <--

@RequestMapping("/times")

public class TimeController {

private final TimeService timeService;

public TimeController(TimeService timeService) {

this.timeService = timeService;

}

@PostMapping

public ResponseEntity<TimeResDto> addTime(@RequestBody TimeDto timeDto) {

TimeResDto timeResDto = timeService.addTime(timeDto);

HttpHeaders headers = createHeader("/times/" + timeResDto.id());

return new ResponseEntity<>(timeResDto, headers, HttpStatus.CREATED);

}

@RestController 어노테이션 내부를 살펴보면 @ResponseBody가 존재하기 때문에 지우면 더 좋을 것 같아요!

앗... 깜박했네요 감사합니다...😅

YehyeokBang · 2024-07-24T17:06:35Z

src/main/java/roomescape/dao/RoomDAO.java

+    public List<Reservation> findAll() {
+        return jdbcTemplate.query("SELECT r.id as reservation_id, r.name, r.date, t.id as time_id, t.time as time_value " +
+                        "FROM reservation as r inner join time as t on r.time_id = t.id",
+                (resultSet, rowNum) -> new Reservation(
+                        resultSet.getInt("reservation_id"),
+                        resultSet.getString("name"),
+                        resultSet.getString("date"),
+                        new Time(resultSet.getInt("time_id"), resultSet.getString("time_value"))
+                ));
+    }


문자열을 더하기 연산으로 나누다 보니 가독성이 조금 떨어지는 것 같아요.

Java Multiline String 포스팅을 참고해서 블럭 형태로 sql 문을 작성하면 더 좋아질 것 같아요!

YehyeokBang · 2024-07-24T17:07:58Z

src/main/java/roomescape/dao/TimeDAO.java

+    public void delete(int id) {
+        jdbcTemplate.update("DELETE FROM time WHERE id = ?", id);
+    }


public void deletebyId(int id) { jdbcTemplate.update("DELETE FROM reservation WHERE id = ?", id); }

준영이 RoomDAO에서 작성한 것처럼 일관성을 위해 deleteById로 변경하면 더욱 명확하게 의도를 전달할 수 있을 것 같아요.

일관성을 위해 그게 더 좋은 것 같네요 감사합니다!

YehyeokBang · 2024-07-24T17:08:45Z

src/main/java/roomescape/domain/Reservation.java

+public class Reservation {
+    private int id;
+
+    private String name;
+    private String date;
+    private Time time;


id와 나머지 필드를 줄바꿈을 통해 구분한 특별한 이유가 있나요?

특별한 이유는 없습니다... 수정하는 게 좋을 것 같습니다!

YehyeokBang · 2024-07-24T17:09:03Z

src/main/java/roomescape/domain/Reservation.java

+
+

불필요한 공백은 제거하면 좋을 것 같아요

공백이 있었네요 감사합니다...! 😲

YehyeokBang · 2024-07-24T17:14:05Z

src/main/java/roomescape/service/ReservationService.java

+@Service
+public class ReservationService {
+    private final RoomDAO roomDAO;
+    private final TimeService timeService;


ReservationService에서 RoomDAO 객체와 TimeService 객체를 주입받는 이유가 다른 이유도 있을 수 있지만, 새로운 예약을 생성할 때 Time을 찾아서 Reservation 객체에 넣고 save() 하기 위한 것 같아요.

다른 Service와 의존 관계를 가지는 것이 어떤 장점과 단점이 있을까요?

추가적으로 Time 객체를 만들어서 Reservation 객체에 추가하여 저장시키는 것보다는 Time의 id값을 DAO에게 전달한다면 sql 쿼리로 충분히 저장할 수 있을 것 같은데 준영은 어떻게 생각하나요?

서비스에서 다른 서비스단을 의존성 주입 받으면 재사용성을 할 수 있다는 장점이 있지만 mvc패턴이 제대로 지켜지지 않아 복잡도가 증가하고 단위 테스트를 할 때도 의존성이 증가해 어려움이 있습니다.
다시 코드를 살펴보니 굳이 Time객체를 만드는 것보다 id의 값으로 받아 전달하는 게 맞는 것 같습니다...!😅 감사합니다

YehyeokBang · 2024-07-24T17:15:02Z

src/main/java/roomescape/service/ReservationService.java

+    @Transactional(readOnly = true)
+    public List<ReservationDto> getAllReservations() {
+        return roomDAO.findAll().stream()
+                .map(reservation -> new ReservationDto(reservation.getName(), reservation.getDate(),
+                        reservation.getTime()))
+                .collect(Collectors.toList());
+    }


Suggested change

@Transactional(readOnly = true)

public List<ReservationDto> getAllReservations() {

return roomDAO.findAll().stream()

.map(reservation -> new ReservationDto(reservation.getName(), reservation.getDate(),

reservation.getTime()))

.collect(Collectors.toList());

}

@Transactional(readOnly = true)

public List<ReservationDto> getAllReservations() {

return roomDAO.findAll().stream()

.map(reservation -> new ReservationDto(reservation.getName(), reservation.getDate(),

reservation.getTime()))

.toList();

}

collect(Collectors.toList()) vs toList() 중에서 전자를 선택하신 이유가 있나요?

Collectors.toList()는 가변 리스트를 반환하지만 toList()는 불변 리스트를 반환합니다. 컬렉션을 수정할 필요가 있는 경우 Collectors를 사용하고 불변성을 유지하고 싶을 경우 toList()를 사용한다고 합니다. 결과값을 불변성으로 보장하기 위해 toList()를 사용하는 것이 좋아보이는 것 같습니다 감사합니다! 😊

collect(Collectors.toList()) 와 toList()의 차이를 사실 잘 모르고 사용했는데 이번 기회에 공부하면서 제대로 알게 된 것 같습니다!

YehyeokBang · 2024-07-24T17:20:05Z

src/main/java/roomescape/service/TimeService.java

+    @Transactional
+    public TimeResDto addTime(TimeDto timeDto) {
+        Time time = new Time(0, timeDto.time());
+        timeDAO.insert(time);
+        int id = timeDAO.getId(time)
+                .orElseThrow(() -> new IllegalArgumentException("Time ID를 찾을 수 없습니다."));
+        return new TimeResDto(id, time.getTime());
+    }


@Transactional 어노테이션을 적용했기 때문에 addTime() 메서드가 하나의 트랜잭션 범위 내에서 실행될 것으로 예상돼요. 만약 DB 작업 중 문제가 생기면 롤백이 이루어질 것 같아요.

새로운 Time 컬럼이 추가되는 작업(insert)만 실행되는 것으로 보여져요. 그렇다면 @Transactional 어노테이션이 있거나 없거나 데이터 삽입 중 실패하면 데이터가 저장되지 않는(또는 롤백되어 그대로인) 상태는 같을 것 같은데 준영이 특별하게 사용한 이유가 있나요? 알려주세요!

@Transactional을 찾아보다 궁금한 점이 생겼습니다! 만약 @Transactional이 없는 경우 데이터베이스 작업이 자동 커밋 모드로 작동할 수 있어, 각 SQL 작업이 독립적으로 커밋되어 timeDAO.insert(time) 작업 후 timeDAO.getId(time) 호출 중 예외가 발생하면, 삽입된 데이터는 그대로 데이터베이스에 커밋된 상태로 남아 있을 수도 있다고 하는데 이게 맞는 건지 궁금합니다.

YehyeokBang

더 명확해진 것 같아요 고생하셨습니다!

Junyeong-An and others added 30 commits June 27, 2024 20:21

1단계 - 홈화면 구성

316a28e

2단계 - 예약 조회

86dcc4d

3단계 - 예약 추가/취소

633f3fa

4단계 - 예외 처리

cc436f5

4단계 - 예외 처리 Reservation추가

9f2966a

테스트 모두 통과하도록 수정

fcfc498

불필요 공백 제거

ddb26a9

Test 코드에 @DisplayName추가

598103c

thymeleaf 의존성 추가

be66739

Feat: 5단계: 데이터베이스 적용하기

08c7387

Feat: 6단계: 데이터 조회하기

6c5bfc9

Feat: 7단계: 데이터 추가/삭제하기

cc73923

Feat: 불필요 코드 삭제

48c3027

Merge branch 'junyeong-an' into junyeong-an

deb6d02

Feat: static 제거

ce883c0

Merge branch 'junyeong-an' of https://github.com/Junyeong-An/spring-r…

e6ad343

…oomescape-playground into junyeong-an

Feat: insert 예외 처리 추가

b7b92b0

Refactor: 테스트 코드명 영어로 변환

c5b300e

Refactor: id값 없을 경우 예외 처리

1a7d4cb

Feat: Dto 추가

520a3db

Feat: Time insert,delete,get 구현

9a92583

Feat: ReservationService 추가

878d9dd

Feat: 9단계 기존 코드 수정

80f0f20

Feat: 10단계 계층화 리팩토링

976c1de

Refactor: 계층화 리팩토링

12561a7

Refactor: 테스트 코드명 수정

7246d92

Refactor: 필요없는 import문 제거

bb9ddc2

Feat: Time 반환 Dto 추가

cadabba

Feat: 테스트 코드 설명 추가

d55847c

Refactor: 예약 서비스에서 Dto 사용추가

4f515e3

YehyeokBang suggested changes Jul 12, 2024

View reviewed changes

sangjin6439 reviewed Jul 13, 2024

View reviewed changes

src/main/java/roomescape/controller/RoomescapeController.java Show resolved Hide resolved

sangjin6439 reviewed Jul 13, 2024

View reviewed changes

src/main/java/roomescape/controller/TimeController.java Show resolved Hide resolved

sangjin6439 reviewed Jul 13, 2024

View reviewed changes

refactor: 계층화 분리

e47f4a0

dd-jiyun approved these changes Jul 15, 2024

View reviewed changes

Junyeong-An added 4 commits July 22, 2024 20:55

remove: set메서드 제거

6d117be

refactor: header 로직 분리

f93e66f

refactor: try-catch 문 제거

cbec7ba

feat: Transactional적용

57bbf2e

Junyeong-An requested a review from YehyeokBang July 24, 2024 10:13

YehyeokBang suggested changes Jul 24, 2024

View reviewed changes

refactor: toList()리팩토링

ee69a26

Junyeong-An requested a review from YehyeokBang July 24, 2024 17:49

YehyeokBang approved these changes Jul 25, 2024

View reviewed changes

Junyeong-An requested a review from sangjin6439 July 25, 2024 13:45

Junyeong-An added 2 commits July 25, 2024 22:47

refactor: 필요없는 공백 및 어노테이션 제거

8c9ff70

refactor: 메서드명 일관성있게 수정

3a3a85b

sangjin6439 approved these changes Jul 26, 2024

View reviewed changes

YehyeokBang approved these changes Aug 2, 2024

View reviewed changes

[Spring Core] 안준영 미션 제출합니다. #306

Are you sure you want to change the base?

[Spring Core] 안준영 미션 제출합니다. #306

Conversation

Junyeong-An commented Jul 11, 2024

구현

고민할 점

YehyeokBang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyeong-An Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dd-jiyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YehyeokBang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyeong-An Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyeong-An Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyeong-An Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YehyeokBang left a comment

Choose a reason for hiding this comment

Junyeong-An Jul 24, 2024 •

edited

Loading

Junyeong-An Jul 25, 2024 •

edited

Loading

Junyeong-An Jul 25, 2024 •

edited

Loading

Junyeong-An Jul 25, 2024 •

edited

Loading