-
Notifications
You must be signed in to change notification settings - Fork 133
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
[Spring Core] 김이화 미션 제출합니다. #309
base: ihwag719
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다~
@Controller | ||
@RequestMapping("/reservations") | ||
public class viewController { | ||
@GetMapping("/reservationpage") | ||
public String reservationPage() { | ||
return "new-reservation"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스 이름은 대문자로 시작하는 것이 관례입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 해당 URI로도 실행했을 때 화면이 표시되나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Autowired | ||
private JdbcTemplate jdbcTemplate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필드 주입을 선택하신 이유가 있나요?
- Spring에서는 생성자 주입을 권장하고 있어요. 그 이유에 대해 학습하고 설명해주세요!
- Spring Docs - Dependency Injection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생성자, 세터, 필드 중 한가지로 의존성을 주입하면 된다고 생각하고 필드 주입을 사용했습니다. 다음부터는 생성자를 통해 의존성을 주입하도록 하겠습니다!
- 생성자 주입은 의존성을 명시적으로 주입하기 때문에 단위 테스트 작성이 쉬워집니다.
- 생성자 주입은 객체 생성 시 필요한 모든 의존성을 명확하게 전달받기 때문에 객체 생성 과정이 더 명확해지고 코드 구조가 단순해져서 유지보수성과 가독성을 향상시키는 데 도움이 됩니다.
- 생성자 주입을 사용하면 의존성을 생성자에서만 주입받기 때문에 주입된 필드를
final
로 선언할 수 있고 객체의 불변성을 유지할 수 있습니다. - 생성자 주입은 컴파일 오류를 발생시켜 순환 의존성을 방지할 수 있습니다. 순환 의존성은 두 개 이상의 빈 또는 클래스가 서로를 직접 또는 간접적으로 의존하고 있는 상황을 말하는데, 이러한 상황에서 의존성을 주입할 때 컴파일 오류를 발생시킵니다.
public List<Reservation> findAllReservations() { | ||
String sql = "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"; | ||
return jdbcTemplate.query(sql, rowMapper); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public List<Reservation> findAllReservations() { | |
String sql = "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"; | |
return jdbcTemplate.query(sql, rowMapper); | |
} | |
public List<Reservation> findAllReservations() { | |
String sql = """ | |
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 | |
"""; | |
return jdbcTemplate.query(sql, rowMapper); | |
} |
- 이런식으로 텍스트 블럭을 사용하면 +연산자 없이 더 깔끔히 선언할 수 있을 것 같아요.
- Java-Multiline-String
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵!!
@PostMapping | ||
public ResponseEntity<Time> createTime(@RequestBody Time time) { | ||
Time newTime = timeService.createTime(time); | ||
return ResponseEntity.created((URI.create("/times/" + newTime.getId()))).body(newTime); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
domain 객체(Time)를 Controller-Service-DAO 모든 계층에서 사용하고 있는 것 같아요. 만약 테이블 구조가 변경되어 domain 클래스가 변경된다면 무슨 문제가 생길 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 객체에서 동일한 도메인 객체(Time)를 사용하면, 테이블 구조가 변경될 때 해당 도메인 클래스도 변경되어야 하는데 이 변경은 모든 계층에 영향을 미치게 되어 각 계층의 코드도 함께 수정해야 합니다. 이렇게 되면 코드의 유연성을 저하시키고 도메인 객체의 변경이 빈번하게 발생하면 코드 수정 및 배포 과정이 복잡해질 수도 있습니다. 또한 테스트 코드도 함께 수정하게 되면서 테스트의 복잡성을 증가시키고 테스트 유지보수 비용도 높이게 됩니다.
즉, 모든 계층이 동일한 도메인 객체에 의존하기 때문에 여러 계층에 전파되어 시스템 결합도를 높이고 코드 유지보수가 어려워집니다. 또한 각 계층은 서로 다른 책임을 가지지만 동일한 도메인 객체를 공유하면 각 계층의 책임이 명확히 분리되지 않아 응집도를 낮추고 코드의 명확성을 떨어뜨립니다. 코드 개선해서 리팩토링하겠습니다!
고생 많으셨습니당 리뷰 간단히 남겼습니다!! |
if (!reservation.getTime().matches("^([01]?[0-9]|2[0-3]):[0-5][0-9]$")) { | ||
throw new InvalidReservationException("time", "시간 형식이 잘못되었습니다."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정규표현식 사용👍🏻
Reservation reservation = new Reservation(null, requestDto.getName(), requestDto.getDate(), time); | ||
Reservation newReservation = reservationDAO.insert(reservation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일부 필드만 있는 생성자를 만들어주는 건 어떨까용? 이게 더 좋은 방법이라면 알려주세욥!!🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일부 필드만 있는 생성자를 만들어서 사용하는 방향으로 리팩토링 했습니다! Reservation 객체를 생성할 때 id 필드를 제외하고 생성함으로써 null 값을 전달받지 않아도 돼서 더 좋은 방법인 것 같네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
시간 관리 페이지 Mapping도 있어야 하지 않나용? 없어도 시간 관리 페이지가 나오는지 궁금합니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Viewcontroller
에 시간 관리 페이지를 작성하지 않아서 추가했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 많으셨습니다~~
|
||
private final TimeDAO timeDAO; | ||
|
||
@Autowired |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Autowired
어노테이션을 생성자에 붙인 이유가 무엇인가요? 만약 붙이지 않으면 생기는 문제가 무엇이 있을까요?
List<Reservation> reservations = reservationDAO.findAllReservations(); | ||
return reservations.stream() | ||
.map(reservation -> new ReservationResponseDto(reservation.getId(), reservation.getName(), reservation.getDate(), reservation.getTime())) | ||
.collect(Collectors.toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.collect(Collectors.toList())
보다 .toList()
를 사용하는 것은 어떨까요? 이 둘의 차이점이 무엇이 있을지 알아보면 좋을 거같아요!
No description provided.