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

[라이브러리 구현하기 - 2단계] 헙크(정현승) 미션 제출합니다. #351

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

HubCreator
Copy link
Member

안녕하세요 에단!

사실 이전 단계에서 기존의 JdbcTemplate을 참고해 구현하다보니 많이 고칠게 없더라구요. 한편으로는 참고하지 않고 조금 더 저만의 방식으로 구현했다면 이번에 얻을 게 조금 더 있었겠다라는 아쉬움이 있네요ㅠㅠ

이번에 조금 더 에단과 얘기를 나눠보고 싶은 것은..

예외 변환을 dao 클래스 쪽에 있는 DataAccessException으로 해도 되는가?

런타임 예외로 변환해주는 것은 좋지만 해당 클래스가 너무 뭉뚱그려진 예외라고 생각해서, 조금 더 구체적인 예외를 던지는 것이 좋지 않을까 생각했습니다. 그런데 또 query, queryForObject, update마다 다른 예외 케이스를 잡기도 어렵더라구요. 이미 하위에서 SQLException을 던지기 때문에 개발자가 catch할 수 있는 것은 이 타입 밖에 없습니다. 그래서 일단은 DataAccessException으로 예외 변환을 해주도록 그대로 두었습니다.

그 외에 트랜잭션 관리 부분은 그 다음 단게의 요구사항인 것 같아서 다음 단계에서 고려해보도록 하겠습니다 :) 감사합니다!

@cookienc
Copy link

cookienc commented Oct 2, 2023

예외 변환을 dao 클래스 쪽에 있는 DataAccessException으로 해도 되는가?

에 대한 답은 저도 헙크와 같은 의견이에요!
모든 메서드 별로 SqlException을 처리하는 예외를 만들면 예외는 명확하겠지만, 클래스가 많아져 관리하기가 어려울것 같아서요.
추가적으로 예외의 목적에 관해서 생각해봤어요. SQL 관련 예외는 클라이언트가 처리할 수 없기 때문에 결국 예외를 보는건 개발자만 보겠죠. 여기서 예외를 메서드 별로 처리하기 vs 하나로 처리하기가 나오는데 앞서 이야기 드렸듯이 예외를 메서드 별로 처리하기는 관리가 어려울 것 같아요. 그러면 뒤에 남은 하나로 처리하기가 남는데, 이러면 예외 내용을 자세히 안알려주니까 불편할 것 같다는 느낌도 드네요. 그런데 어차피 stacktrace가 나오니까 괜찮을것(?) 같기도 해요! 그래도 '예외를 개발자에게 친절히 던지고 싶다!'면 저라면 log를 찍는 방법을 선택할 것 같습니다.

Copy link

@cookienc cookienc left a comment

Choose a reason for hiding this comment

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

안녕하세요! 헙크 :) 이제 이틀 뒤면 출근해야 하네요ㅠㅠ
1단계 때 다하셔서 이번엔 코드 리뷰할 게 진짜로 없네요ㅋㅋㅋ
바로 머지하겠습니다! 질문에 대한 저의 의견은 코멘트로 남겨 놓았습니다.
그럼 남은 연휴도 잘보내세요😄

* 이는 일반적으로 잘못된 ResultSet 열 인덱스 또는 이름이 지정되었을 때 발생합니다.
* 또한 연결이 끊긴 SqlRowSets에 의해 발생되기도 합니다.
*/
public class InvalidResultSetAccessException extends RuntimeException {
Copy link

Choose a reason for hiding this comment

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

사용하지 않는데 추가되었네용

@cookienc cookienc merged commit ef9baaa into woowacourse:hubcreator Oct 2, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants