Conversation
leeyohan93
left a comment
leeyohan93
left a comment
There was a problem hiding this comment.
안녕하세요 박설님! 😄
2단계 미션을 잘 구현해주셨습니다!
쿼리 빌더가 난이도가 높은 미션 인데 끝까지 힘내주셔서 감사해요! 💯
개선해보면 좋을 만한 부분에 코멘트를 남겨두었으니 확인부탁드리고
어려운 점이나 궁금한 점이 있으면 DM으로 많이 물어봐주세요! 감사합니다 🙏
| @Test | ||
| void createDDL() throws | ||
| InvocationTargetException, | ||
| NoSuchMethodException, | ||
| InstantiationException, | ||
| IllegalAccessException { | ||
| CreateQueryBuilder createQueryBuilder = new CreateQueryBuilder(); | ||
| String createTableSql = createQueryBuilder.getCreateTableSql(); | ||
|
|
||
| assertEquals(createTableSql, "CREATE TABLE person (id INT AUTO_INCREMENT PRIMARY KEY, name VARCHAR(50) NOT NULL, age INT)"); | ||
| } |
There was a problem hiding this comment.
사소하지만 테스트 시에는 아래와 같이 예외의 상위 클래스인 Exception을 사용하면 잘 정리할 수 있습니다!
@Test
void createDDL() throws Exception {
그리고 Person 클래스를 선언할 때 아래와 같이 데이터베이스에 사용되는 테이블 명이나 컬럼 명을 설정 해주었었는데요!
아래와 같이 쿼리 문에 함께 반영되면 좋을 것 같습니다! 😄
CREATE TABLE users (
id BIGINT AUTO_INCREMENT PRIMARY KEY,
nick_name VARCHAR(255),
old INTEGER,
email VARCHAR(255) NOT NULL
);
|
|
||
| import domain.Person; | ||
|
|
||
| public class DropQueryBuilder { |
There was a problem hiding this comment.
DropQueryBuilder 에 대한 테스트도 함께 작성되어도 좋을 것 같아요 😄
|
|
||
| Class<Person> personClass = Person.class; | ||
|
|
||
| Person person = personClass.getConstructor().newInstance(); |
| public String getCreateTableSql() throws | ||
| NoSuchMethodException, | ||
| InvocationTargetException, | ||
| InstantiationException, | ||
| IllegalAccessException { | ||
|
|
||
| Class<Person> personClass = Person.class; |
There was a problem hiding this comment.
지금은 해당 쿼리를 만드는 메서드의 기능이 Person 클래스에 한정되어있는데요!
파라미터로 클래스를 받아서 다른 클래스에 대해서도 기능이 동작하도록 개선해보는건 어떨까요!?
public String getCreateTableSql(Class<?> clazz) {
// 내부 구현..
}
| // DB에서 가져와야겠다 ai를 // | ||
| private static String generateValue() { | ||
| return "AUTO_INCREMENT"; | ||
| } |
There was a problem hiding this comment.
데이터베이스 마다 해당 기능에 대한 쿼리가 다를 것이라고 생각하고 고민해주신 것 같아요! 👍 👍
다만 지금은 하나의 DB 라고 생각하고 구현 범위를 좁혀서 개발해보시는 것을 추천드립니다!
그렇다면 해당 문자열은 아래 처럼 상수로 만들어 사용할 수도 있겠네요!
private static final AUTO_INCREMENT_QUERY = "AUTO_INCREMENT"
getPrimaryKey()도 동일한 피드백입니다! 😄
| private static String isNull(boolean isNullable) { | ||
| if (!isNullable) { | ||
| return "NOT NULL"; | ||
| } | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
static을 제거하고 사용해도 좋을 것 같아요! getDeclaredFields() 도 동일한 피드백입니다!
| private static String isNull(boolean isNullable) { | |
| if (!isNullable) { | |
| return "NOT NULL"; | |
| } | |
| return ""; | |
| } | |
| private String isNull(boolean isNullable) { | |
| if (!isNullable) { | |
| return "NOT NULL"; | |
| } | |
| return ""; | |
| } |
| // email VARCHAR(50) NOT NULL, | ||
| Field emailField = Arrays.stream(getDeclaredFields(personClass)) | ||
| .filter(x -> x.isAnnotationPresent(Column.class)) | ||
| .filter(x -> !x.getAnnotation(Column.class).nullable()) | ||
| .findFirst().get(); | ||
| String email = emailField.getName(); | ||
| String emailType = javaClassTypeToDbTypes.get(emailField.getType()); | ||
| boolean isNullable = emailField.getAnnotation(Column.class).nullable(); | ||
|
|
||
| String emialNullable = isNull(isNullable); | ||
|
|
||
| String emailCombi = String.format("%s %s %s", email, emailType, emialNullable); |
There was a problem hiding this comment.
구현해주신 여러 필드로 쿼리를 만들때 모두 아래와 같은 동일한 과정을 거치고 있는데요!
각각의 필드를 별도로 수행하지 않고 함께 추상화해서 기능을 수행할 수 있도록 개선해보면 좋을 것 같아요! 🙏
- 클래스의 모든 필드를 조회한다.
- 조회한 필드 중
@Column이 선언된 필드를 조회한다. @Transient를 포함하는 필드를 제외한다- 필드의 이름, 타입, 옵션(nullalbe 여부 등) 정보를 조회한다.
- 쿼리를 만든다.
2 participants
코드 리뷰 감사합니다.