-
Notifications
You must be signed in to change notification settings - Fork 71
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단계 - QueryBuilder DDL #225
base: parkseoldev
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.
안녕하세요 박설님! 😄
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사소하지만 테스트 시에는 아래와 같이 예외의 상위 클래스인 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DropQueryBuilder
에 대한 테스트도 함께 작성되어도 좋을 것 같아요 😄
|
||
Class<Person> personClass = Person.class; | ||
|
||
Person person = personClass.getConstructor().newInstance(); |
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 String getCreateTableSql() throws | ||
NoSuchMethodException, | ||
InvocationTargetException, | ||
InstantiationException, | ||
IllegalAccessException { | ||
|
||
Class<Person> personClass = Person.class; |
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.
지금은 해당 쿼리를 만드는 메서드의 기능이 Person
클래스에 한정되어있는데요!
파라미터로 클래스를 받아서 다른 클래스에 대해서도 기능이 동작하도록 개선해보는건 어떨까요!?
public String getCreateTableSql(Class<?> clazz) {
// 내부 구현..
}
// DB에서 가져와야겠다 ai를 // | ||
private static String generateValue() { | ||
return "AUTO_INCREMENT"; | ||
} |
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.
데이터베이스 마다 해당 기능에 대한 쿼리가 다를 것이라고 생각하고 고민해주신 것 같아요! 👍 👍
다만 지금은 하나의 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구현해주신 여러 필드로 쿼리를 만들때 모두 아래와 같은 동일한 과정을 거치고 있는데요!
각각의 필드를 별도로 수행하지 않고 함께 추상화해서 기능을 수행할 수 있도록 개선해보면 좋을 것 같아요! 🙏
- 클래스의 모든 필드를 조회한다.
- 조회한 필드 중
@Column
이 선언된 필드를 조회한다. @Transient
를 포함하는 필드를 제외한다- 필드의 이름, 타입, 옵션(nullalbe 여부 등) 정보를 조회한다.
- 쿼리를 만든다.
코드 리뷰 감사합니다.