-
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
[1.1.0/AN_FEAT] 배틀 구도 날씨 저장 #343
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.
수고하셨습니다 오둥 👍
android/data/src/main/java/poke/rogue/helper/data/repository/BattleRepository.kt
Show resolved
Hide resolved
android/data/src/main/java/poke/rogue/helper/data/repository/DefaultBattleRepository.kt
Outdated
Show resolved
Hide resolved
.../app/src/main/java/poke/rogue/helper/presentation/battle/view/WeatherItemSelectedListener.kt
Show resolved
Hide resolved
android/data/src/main/java/poke/rogue/helper/data/repository/DefaultBattleRepository.kt
Show resolved
Hide resolved
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.
@JoYehyun99 코멘트 답변 남겨두었습니다잇!
android/data/src/main/java/poke/rogue/helper/data/repository/DefaultBattleRepository.kt
Show resolved
Hide resolved
/noti 날씨 저장되는거 PR 코드리뷰해주세요요요오오오ㅗ옹 |
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.
LGTM!~
좋아용~
수고많으셨슴둥 오둥~
android/app/src/main/java/poke/rogue/helper/presentation/battle/BattleActivity.kt
Show resolved
Hide resolved
android/app/src/main/java/poke/rogue/helper/presentation/battle/BattleActivity.kt
Show resolved
Hide resolved
android/app/src/main/java/poke/rogue/helper/presentation/battle/BattleViewModel.kt
Show resolved
Hide resolved
import android.view.View | ||
import android.widget.AdapterView | ||
|
||
inline fun <reified T> itemSelectListener(crossinline onSelected: (T) -> Unit): AdapterView.OnItemSelectedListener { |
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.
crossinline
으로 해야 할 이유가 있나요?
비지역 반환을 방지해야하는 게 맞나..?
crossinline
한 번도 안 해봐서 잘 모르겠슴둥
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.
@sh1mj1 저는 no-inline 람다 블록에서 inline 람다를 호출할 필요성이 있을 때 crossinline
키워드를 사용합니다!
그리고, 다른 함수로 람다식을 argument로서 전달해줘야하는 상황이라면 noinline
을 사용합니다 ㅎㅎ
여담으로 inline 키워드를 활용하면 비지역 반환을 사용할 수 있긴 합니다만, 비지역 반환을 선호하지 않습니다.
사용할 일도 크게 없을 뿐 더러, 무분별하게 사용할 경우 코드를 읽는 사람 입장에서 헷갈리기 때문입니다.
가끔 코테 풀때 repeat{} 내부에서 비지역 반환을 사용한 경우 말고 사용한 적이 없네요 😉
심지는 어떨 때 비지역 반환을 사용하시는지 궁금하네요! 😁
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.
고생하셨습니다 굿 5둥2 👍
android/data/src/main/java/poke/rogue/helper/data/datasource/LocalBattleDataSource.kt
Show resolved
Hide resolved
override suspend fun savePokemon(pokemonId: String) { | ||
localBattleDataSource.savePokemon(pokemonId) | ||
} | ||
override suspend fun savePokemon(pokemonId: String) = localBattleDataSource.savePokemon(pokemonId) |
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.
이거는 왜 다시 반환하도록 수정되었나요 ?!?
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.
어,,? 그러게요
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.
아 예니가 아래에 =
을 사용했길래 통일성 맞추려고 사용했던 것 같습니다!
/noti 마지막 코드리뷰 완 ! |
작업 영상
2024-09-26.10.51.08.mov
작업한 내용
xxStream()
으로 컨벤션 결정 -> Flow 로 다 네이밍 변경했습니다