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

Step1: EntityLoader 구현 #6

Merged
merged 4 commits into from
Jul 11, 2023
Merged

Step1: EntityLoader 구현 #6

merged 4 commits into from
Jul 11, 2023

Conversation

ghojeong
Copy link
Member

@ghojeong ghojeong commented Jul 8, 2023

예비군 훈련을 다녀오느라 PR 이 늦었습니다.
ResultSetMetaData 를 활용해 EntityLoader 를 구현해보았습니다.

지난 미션에서 마지막에 주셨던 피드백을 아직 제대로 반영하지 못했지만,
우선 구현한데까지라도 이렇게 PR 을 올립니다.

Comment on lines +31 to +44
public T mapRow(ResultSet resultSet) {
try {
final T object = clazz.getDeclaredConstructor().newInstance();
final ResultSetMetaData metaData = resultSet.getMetaData();
final int columnCount = metaData.getColumnCount();
for (int i = 1; i <= columnCount; i++) {
fieldsByName.get(
metaData.getColumnLabel(i)
).set(
object,
resultSet.getObject(metaData.getColumnName(i))
);
}
return object;
Copy link
Member Author

Choose a reason for hiding this comment

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

ResultSetMetaData 위주로 EntityLoader 를 구현해보았습니다.

Choose a reason for hiding this comment

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

동적으로 구현 잘하셨군요! 크

Copy link

@liquidjoo liquidjoo left a comment

Choose a reason for hiding this comment

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

1단계 미션 구현 및 피드백 반영 잘하셨어요
�몇가지 코멘트 남겼어요 다음 단계에 반영해 보면 좋을 것 같아요!
다음 단계 진행해 주세요 🙇

Comment on lines +11 to 15
if (entityManager.isNew(entity)) {
entityManager.persist(entity);
} else if (entityManager.isDirty(entity)) {
entityManager.merge(entity);
}

Choose a reason for hiding this comment

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

else-if 를 사용하지 않고 구현할 수 있을 것 같은데요~
도전해보아도 좋을 것 같습니다 :)

Comment on lines +20 to +27
this.fieldsByName = ColumnFields.forQuery(clazz)
.stream().collect(Collectors.toMap(
ColumnName::build,
field -> {
field.setAccessible(true);
return field;
}
));

Choose a reason for hiding this comment

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

메서드 분리를 해보아도 좋을 것 같아요!

Comment on lines +31 to +44
public T mapRow(ResultSet resultSet) {
try {
final T object = clazz.getDeclaredConstructor().newInstance();
final ResultSetMetaData metaData = resultSet.getMetaData();
final int columnCount = metaData.getColumnCount();
for (int i = 1; i <= columnCount; i++) {
fieldsByName.get(
metaData.getColumnLabel(i)
).set(
object,
resultSet.getObject(metaData.getColumnName(i))
);
}
return object;

Choose a reason for hiding this comment

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

동적으로 구현 잘하셨군요! 크

Comment on lines +33 to +35
final String query = isNew(entity)
? dml.getInsertQuery(entity)
: dml.getUpdateQuery(entity);

Choose a reason for hiding this comment

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

삼항 연산자도 좋으나 코드의 가시성을 위해 분리해보아도 좋을 것 같습니다

@liquidjoo liquidjoo merged commit 141549c into next-step:ghojeong Jul 11, 2023
ghojeong added a commit to ghojeong/jpa-association that referenced this pull request Aug 16, 2024
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