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

Seowoo #4

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Seowoo #4

wants to merge 3 commits into from

Conversation

dltjdn
Copy link

@dltjdn dltjdn commented Mar 27, 2023

No description provided.

Copy link
Contributor

@jinsu4755 jinsu4755 left a comment

Choose a reason for hiding this comment

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

일단 반드시 보면 좋을 것 같은 부분 리뷰남겼습니다. 반영해주세요~!

현재 작성중인 JPA Entity에 생성이 원하는 대로 이루어지는지 실제로 확인을 할 수 있다면 좋을 것 같습니다.
남은 부분에 대해서는 Test 코드를 작성해서 실제로 원하는 동작을 할 수 있는지 확인해 보는 것이 좋을 것 같습니다.

Comment on lines +13 to +36
class Users(
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "id", nullable = false)
var id: Int = 0,

@Column(name = "sns_token", nullable = false)
var snsToken: String,

@Column(name = "sns_type", nullable = false)
var snsType: String,

@Column(name = "user_email", nullable = false)
var userEmail: String,

@Column(name = "profile_image")
var profileImage: String,

@Column(name = "created_at")
var createdAt: LocalDateTime = LocalDateTime.now(),

@Column(name = "updated_at")
var updatedAt: LocalDateTime = LocalDateTime.now()
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Kotlin 에서는 var 로 선언한 프로퍼티는 자바의 맴버 변수, getter, setter 와 동일한 기능을 합니다. 또한 이를 주 생성자의 파라미터로 둔다면 생성자 또한 생성되게 됩니다.

만약 해당 객체를 생성한 후 임의로 값을 변경할 수 있다면 어떤 일이 발생할까요?

Comment on lines +10 to +11
@Column(name = "content", nullable = false)
var content: String? = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

컬럼 자체에서는 nullable = false 로 null이 들어갈 수 없는 상태로 선언 되었지만 해당 객체는 생성 시 null이 Default Value 로 들어가게 됩니다.

해당 부분에서 발생하는 문제가 있는지 확인해보면 좋을 것 같습니다.

Comment on lines +21 to +22
@Column(name = "profile_image")
var profileImage: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Pet은 등록 시 사진이 없을 수 있는 객체이지 않나요?

만약 현재 Pet Entity 를 생성한다면 profileImage를 받지 않은 경우라면 Pet을 생성조차 못하는 상황으로 보입니다.

Kotlin 에서는 자바와 다르게 Null이 들어갈 수 있는 타입과, Null이 들어갈 수 없는 타입으로 나뉩니다.
String? 과 같은 타입으로 사용하면 Null이 들어갈 수 있는 String 이고 String 으로만 쓴다면 Null이 들어갈 수 없는 String 입니다.

Kotlin 에서는 왜 이것을 나누어 사용하는지 알아보면 좋을 것 같습니다~

Comment on lines +35 to +37
@ManyToMany
@JoinTable(name = "pet_memory")
val memories: List<Memory>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Pet의 profileImage 와 동일하게 생성 시 memories 는 항상 그 값을 초기화 하도록 선언되어, 언제나 빈 리스트가 될 것 같은데 이 경우에는 Default 값을 사용해주는 것이 어떨까요?

@jinsu4755 jinsu4755 marked this pull request as draft March 27, 2023 20:16
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