-
Notifications
You must be signed in to change notification settings - Fork 0
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
모델 확장 함수 생성 #314
모델 확장 함수 생성 #314
Conversation
일단 Lecture 확장 함수만 추가해봤는데 |
SATURDAY, | ||
SUNDAY | ||
enum class Day(toInt: Int) { | ||
MONDAY(0), | ||
TUESDAY(1), | ||
WEDNESDAY(2), | ||
THURSDAY(3), | ||
FRIDAY(4), | ||
SATURDAY(5), | ||
SUNDAY(6), | ||
} |
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.
이거 이렇게 하는거랑 ordinal 쓰는거 중에 뭐 하나가 안좋은 방법이라고 했던것 같은데 뭐였지
살짝 보니까 나중에 ui에서도 그렇고
지금 확장 함수 옮겨올 때도 그렇고 저게 유용하게 쓰일것 같아서
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.
ordinal 쓰는게 안좋은방법이야 선언 순서에 의존하게되는거니까
https://kukim.tistory.com/69
아니면 fromInt() 같은걸 만드는건어때? 지금 보니까 ClassTimeDto.toExternalModel()에서 0~6값을 변환해주는게 다라서 toInt 자체를 다른곳에서 쓰진 않길래
나중에 UI에서 쓴다고하면.. 그때 Day 객체들의 list 만들어서 쓰는 걸로 충분할거같기도 해 지금 DayTimePickerSheet의 dayList처럼
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.
그리고 클래스 생성자의 파라미터에 val 안붙이면 생성자의 파라미터일 뿐이지, 프로퍼티가 되진 않아
다른 함수에서 Day.MONDAY.toInt 해보면 unresolved reference 떠
자바의 생성자에서 this.a = a
번거롭게 해줘야 했던 걸 줄여준 문법
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.
아 Lectures.contains()에서 ordinal 쓰는구나
그러게 프로퍼티 하나 갖고있으면 좋겠다
// TODO : context가 필요한 확장 함수는 어떻게 하지? | ||
//fun Lecture.getQuotaTitle(context: Context): String = StringBuilder().apply { | ||
// append(context.getString(R.string.lecture_detail_quota)) | ||
// if (freshmanQuota != null && freshmanQuota != 0L) append("(${context.getString(R.string.lecture_detail_senior)})") | ||
//}.toString() | ||
|
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.
이 확장함수는 context를 필요로 하는데 어떻게 해야하지..?
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.
요건 model에 있을필요가 없을듯? 지금 하고있는건 변환함수지 확장함수가 아니니까
봤겠지만 SNUTTStringUtils에 있던 유틸 함수고, 강의 상세 페이지에서 정원 항목 title로 정원
또는 정원(재학생)
둘 중 하나를 고르기 위한 함수네
결국 UI의 로직인거고, feature:lecturedetail 같은데에 있으면 될듯?
// TODO : quota랑 freshmanQuota가 open val이면 null 관련 오류 발생 | ||
fun OriginalLecture.getFullQuota(): String = StringBuilder().apply { | ||
append(quota) | ||
if (quota != null && freshmanQuota != null && freshmanQuota != 0L) { | ||
append("(${quota - freshmanQuota})") | ||
} | ||
}.toString() | ||
|
||
fun TimetableLecture.getFullQuota(): String = StringBuilder().apply { | ||
append(quota) | ||
if (quota != null && freshmanQuota != null && freshmanQuota != 0L) { | ||
append("(${quota - freshmanQuota})") | ||
} | ||
}.toString() |
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.
이거 원래는 Lecture의 확장함수로 만들어야 하는데
그러면 Lecture의 quota랑 freshmanQuota가 open val이라서
if 부분에서 null 검사를 해도 또 !!를 달아야 하는 상황이 발생해서 (안달면 오류 발생)
저렇게 두개로 쪼개야 할까
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.
일단 얘도 feature 모델에 있으면 되는 UI로직이긴 한듯
open val은 backing field가 따로 없고, 자식 클래스가 지 맘대로 override 할 수 있어서 smart check가 안돼
예를들어서 TimetableLecture.freshmanQuota를 이딴식으로 정의하면, get 할때마다 null이었다가 notnull이었다가 할수있음. 그럼 Lecture.getFullQuota()를 불렀을 때 Lecture 객체가 TimetableLecture 객체일 경우, 첫 번째 get()에서 notnull이더라도 두 번째 get()에서 null일 수 있기 때문에 smart check가 불가능함 --> 미리 컴파일타임에 에러를 낸다
override val freshmanQuota: Long?
get() = if (Random.nextBoolean()) null else 10L
https://stackoverflow.com/questions/41086296/smartcast-is-impossible-because-property-has-open-or-custom-getter
https://kotlinlang.org/docs/properties.html#backing-fields
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.
이 경우에는 이렇게 할수있을듯? null이면 early return해버리고, null이 아니면 별도 변수에 담아서 쓰는거지
fun Lecture.getFullQuota(): String = StringBuilder().apply {
val quota = quota ?: return@apply
append(quota)
val freshmanQuota = freshmanQuota ?: return@apply
if (freshmanQuota == 0L) return@apply
append("(${quota - freshmanQuota})")
}.toString()
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.
1년전에 주현이형이 똑같은말 했었네ㅋㅋㅋㅋ
#179 (comment)
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.
open val이 backing field가 없는건 아니네ㅋㅋㅋ미안 java로 디컴파일해보니까 backing field 있다
backing field의 존재 여부는 open 여부에 관계없네
프로퍼티의 backing field가 만들어지지 않는 경우
- val 프로퍼티
field
가 등장하지 않는 커스텀 getter를 사용
- var 프로퍼티
field
가 등장하지 않는 커스텀 getter와 setter를 사용할 때
사실당연하네 open은 상속가능하다는 것 뿐이지, backing field가 존재하냐 마냐 얘기가 아니니까
|
||
if (queryDay != placeTime.timetableBlock.day.ordinal) continue // TODO : ordinal? | ||
if (queryTime in start..end) return true |
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.
이거 Time.timeInMinutes 쓸 필요 없이 Time이 Comparable 구현하게 해서 부등호 바로 쓰면 좋을듯?? 해서 푸시해둘게
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.
…extensions # Conflicts: # core/data/src/main/java/com/wafflestudio/snutt2/core/data/model/TimeTable.kt # core/model/src/main/java/com/wafflestudio/snutt2/core/model/data/PlaceTime.kt # core/model/src/main/java/com/wafflestudio/snutt2/core/model/data/lecture/Lecture.kt
아직 우리가 모델 설계도 안 끝났고, 그 모델이 어떻게 사용되는지 아직 모르는데 확장 함수를 벌써 만들 필요가 있을까? 싶음 |
새로 설계한 모델에 맞춰서
기존의 앱 곳곳에 흩어져 있는 확장 함수 옮겨오기