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

[refactor] : Survey 도메인을 kotlin으로 리팩토링한다 #395

Merged
merged 28 commits into from
Mar 12, 2024

Conversation

devxb
Copy link
Member

@devxb devxb commented Mar 7, 2024

어떤 기능을 개발했나요?

Survey 도메인을 kotlin으로 리팩토링 했습니다

어떻게 해결했나요?

  • Survey 어그리거트 루트 -> Target
    기존 도메인 열어보니 Target하나에 여러개의 Survey가 되는거로 구성되어있어서 어그리거트 루트를 Target으로 지정했습니다.
  • Feedback 어그리거트 루트 -> Feedback
  • IdGenerator와 같은 복잡한 로직 삭제 (밖에서 넣어주는게 이해하기 편할듯) 했습니다.

이슈 넘버

참고자료

@devxb devxb self-assigned this Mar 7, 2024
@devxb devxb changed the base branch from main to devxb/survey-kotlin March 7, 2024 08:18
@devxb devxb closed this Mar 8, 2024
@devxb devxb reopened this Mar 8, 2024
Copy link
Member

@dojinyou dojinyou left a comment

Choose a reason for hiding this comment

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

일단은 기존 도메인에 있던 메서드들 다 있어야될 것 같음.

src/build.gradle Outdated Show resolved Hide resolved
src/main/kotlin/domain/feedback/Bookmark.kt Outdated Show resolved Hide resolved
Comment on lines 20 to 21
fun impossible(): domain.feedback.Bookmark =
domain.feedback.Bookmark(bookmarkedAt = TimeUtil.toInstant())
Copy link
Member

Choose a reason for hiding this comment

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

뭐하는 함수인지 모르겠음.

Copy link
Member Author

Choose a reason for hiding this comment

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

지금 정책이 ChoiceFormQuestion (객관식 질문) 에는 북마크할 수 없음. 그래서 이거 표현하려고 해놨어
기능에서 큰 의미가 있지는 않은데, 정책 표현 메소드

Copy link
Member

Choose a reason for hiding this comment

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

음... 잘 모르겠음 표현이 되는건지?
ChoiceFormQuestion는 북마크 할 수 없다.를 어디서 알 수 있는거임?

Copy link
Member Author

Choose a reason for hiding this comment

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

ChoiceFormQuestion에서 이렇게 표현하고 있긴한데, impossible이 정의된 Bookmark랑 사용하는위치가 멀어서 잘 안보이는건가?

스크린샷 2024-03-10 오전 10 17 24

Copy link
Member

Choose a reason for hiding this comment

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

null 넣고 bookmark 관련 메서드들 호출되면 무시하거나 에러 발생하게 하는게 더 좋을 것 같은데?

src/main/kotlin/domain/feedback/Bookmark.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/feedback/Feedback.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/survey/ChoiceFormQuestion.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/survey/FormQuestionable.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/survey/Survey.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/survey/Target.kt Outdated Show resolved Hide resolved
src/main/kotlin/domain/survey/value/ImageUrl.kt Outdated Show resolved Hide resolved
gradle/migration.gradle Show resolved Hide resolved
src/main/kotlin/me/nalab/api/core/TimeBaseEntity.kt Outdated Show resolved Hide resolved
src/main/kotlin/me/nalab/api/core/TimeUtil.kt Outdated Show resolved Hide resolved
) {

companion object {
fun empty(): ImageUrl = ImageUrl("")
Copy link
Member

Choose a reason for hiding this comment

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

요거는 왜!?

Copy link
Member Author

Choose a reason for hiding this comment

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

이거도 지금 정책이 "이미지가 없으면 직군별 기본 이미지" 를 내려주는거라서 empty로 만들어놨으

Copy link
Member

Choose a reason for hiding this comment

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

nullable하게 처리하는 건 별루?

Copy link
Member

@dojinyou dojinyou left a comment

Choose a reason for hiding this comment

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

기존 코멘트 내용 빼고 요것만 반영하면 될듯

Comment on lines 13 to 16
var current = Instant.now()
if (clock != null) {
current = Instant.now(clock)
}
Copy link
Member

Choose a reason for hiding this comment

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

val로 쓰고 if statement 로 처리하기

Suggested change
var current = Instant.now()
if (clock != null) {
current = Instant.now(clock)
}
val current = if (clock == null) Instant.now() else Instant.now(clock)

Copy link
Member Author

Choose a reason for hiding this comment

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

return Instant.parse(formatter.format(instant))
}

fun fixed(clock: Clock?) {
Copy link
Member

Choose a reason for hiding this comment

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

fixed 할 때는 not null type으로 받기

Suggested change
fun fixed(clock: Clock?) {
fun fixed(clock: Clock) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 21 to 22
val formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSSSSX")
.withZone(ZoneId.of("UTC"))
Copy link
Member

Choose a reason for hiding this comment

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

요건 매번 생성하지 말고 기본 필드로 가지고 있어도 될듯!

Copy link
Member Author

Choose a reason for hiding this comment

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


private var clock: Clock? = null

fun toInstant(): Instant {
Copy link
Member

Choose a reason for hiding this comment

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

now 메서드명 그대로 쓰기?!

Suggested change
fun toInstant(): Instant {
fun now(): Instant {

Copy link
Member Author

Choose a reason for hiding this comment

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


@PrePersist
fun prePersist() {
val now = Instant.now()
Copy link
Member

Choose a reason for hiding this comment

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

TimeUtil.now() 사용하기

Copy link
Member Author

Choose a reason for hiding this comment

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


@PreUpdate
fun preUpdate() {
updatedAt = Instant.now()
Copy link
Member

@dojinyou dojinyou Mar 11, 2024

Choose a reason for hiding this comment

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

ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

@devxb devxb merged commit b3ddaa6 into devxb/survey-kotlin Mar 12, 2024
1 check passed
@devxb devxb deleted the devxb/iss-# branch March 12, 2024 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor]: survey 도메인 kotlin 리팩토링
2 participants