-
Notifications
You must be signed in to change notification settings - Fork 33
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
부산대 Android_이창욱_2주차_과제 #33
base: ichanguk
Are you sure you want to change the base?
Conversation
- repository 설명 추가 - 1단계 기능 리스트 추가
검색어 입력 및 검색 결과를 표시할 기본 레이아웃 구현
SQLite를 이용해 로컬 DB에 데이터를 저장하는 기능 구현
2단계 기능 리스트 추가
- 검색어를 입력하면 검색 결과 목록이 표시되도록 함 - MVVM 패턴과 data binding을 사용함
- 검색 결과 목록에서 항목을 누르면 검색어 저장 목록에 추가되도록 함.
clear 버튼을 누르면 해당 저장된 검색어가 목록에서 삭제되도록 구현
기능 리스트에 기능 추가
검색 결과에서 선택한 검색어가 이미 저장된 목록에 있을 경우, 기존 검색어를 삭제하고 다시 추가하도록 구현
이름이 같은 장소가 있을 수 있기 때문에 placeId를 참조하여 저장 및 삭제하도록 구현
|
||
class MainActivity : AppCompatActivity() { | ||
private lateinit var binding: ActivityMainBinding | ||
private val placeViewModel by lazy { ViewModelProvider(this)[PlaceViewModel::class.java] } | ||
private val savedSearchWordViewModel by lazy { ViewModelProvider(this)[SavedSearchWordViewModel::class.java] } |
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.
by viewModels()
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.
멘토링 때에도 말씀드렸다시피 전략적인 선택을 하면 됩니다. 다만 지금 현재 화면의 요구사항이 굉장히 단순하기 때문에 저는 fragment 로 따로 분리하지는 않을 것 같아요. 확장요구에 대한 낌새가 보일 때 fragment 로 분리하는 작업을 진행할 것 같습니다.
또한 같은 Fragment 혹은 Activity 내에서 ViewModel 을 여러개 사용하는 것은 관점에 따라 좋을 수도 나쁠 수도 있습니다. 위에도 언급했다시피 이 화면 자체의 요구사항이 매우 간단하기 떄문에 ViewModel 을 나누기 보다 저는 하나의 ViewModel 로 진행할 것 같습니다 :)
) | ||
} | ||
} | ||
binding.searchResultRecyclerView.adapter = ResultRecyclerViewAdapter(placeItemClickListener) |
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.
Memory Leak, Garbage Collection
*/ | ||
@SuppressLint("NotifyDataSetChanged") | ||
fun setPlaces(places: List<Place>) { | ||
searchResultList = places |
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.
DiffCallabck
import android.database.sqlite.SQLiteOpenHelper | ||
import campus.tech.kakao.map.model.Place | ||
|
||
class PlaceRepository(context: Context) : SQLiteOpenHelper(context, DATABASE_NAME, null, DATABASE_VERSION) { |
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.
withContext(Dispatchers.IO) { | ||
repository.deleteSearchWordById(searchWord.id) | ||
val currentList = _savedSearchWords.value ?: emptyList() | ||
_savedSearchWords.postValue(currentList - searchWord) | ||
} |
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.
저장된 검색어 목록에 데이터를 추가할 때 name이 중복된 경우를 생각해서 place의 id값을 참조해서 사용했는데 올바르게 사용했는지 궁금합니다.
name 이 primary 가 될 수 없다는 말씀이시죠? 잘하셨습니다 :)
|
||
class MainActivity : AppCompatActivity() { | ||
private lateinit var binding: ActivityMainBinding | ||
private val placeViewModel by lazy { ViewModelProvider(this)[PlaceViewModel::class.java] } | ||
private val savedSearchWordViewModel by lazy { ViewModelProvider(this)[SavedSearchWordViewModel::class.java] } |
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.
멘토링 때에도 말씀드렸다시피 전략적인 선택을 하면 됩니다. 다만 지금 현재 화면의 요구사항이 굉장히 단순하기 때문에 저는 fragment 로 따로 분리하지는 않을 것 같아요. 확장요구에 대한 낌새가 보일 때 fragment 로 분리하는 작업을 진행할 것 같습니다.
또한 같은 Fragment 혹은 Activity 내에서 ViewModel 을 여러개 사용하는 것은 관점에 따라 좋을 수도 나쁠 수도 있습니다. 위에도 언급했다시피 이 화면 자체의 요구사항이 매우 간단하기 떄문에 ViewModel 을 나누기 보다 저는 하나의 ViewModel 로 진행할 것 같습니다 :)
if (searchText.isNotEmpty()) { | ||
placeViewModel.searchPlacesByCategory(searchText) | ||
} else { | ||
placeViewModel.searchPlacesByCategory("") | ||
} |
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.
요거 굳이 if 문으로 나눌 필요가 없어 보이는 로직인 것 같아요. searchText 가 empty 라면 "" 일 테니까요!
fun searchPlacesByCategory(category: String) { | ||
viewModelScope.launch { | ||
withContext(Dispatchers.IO) { | ||
val results = placeRepository.getPlacesByCategory(category) | ||
_searchResults.postValue(results) | ||
} | ||
} | ||
} |
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.
Dispatchers.IO 처리는 잘해주셨어요. 다만 withContext 로 처리하기 보다는 launch 시점에 context 를 설정하는 것이 어떨까요?
fun searchPlacesByCategory(category: String) { | |
viewModelScope.launch { | |
withContext(Dispatchers.IO) { | |
val results = placeRepository.getPlacesByCategory(category) | |
_searchResults.postValue(results) | |
} | |
} | |
} | |
fun searchPlacesByCategory(category: String) { | |
viewModelScope.launch(Dispatchers.IO) { | |
val results = placeRepository.getPlacesByCategory(category) | |
_searchResults.postValue(results) | |
} | |
} |
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.
더불어 viewModelScope 은 launch 시에 아무것도 설정하지 않으면 어떤 쓰레드에서 동작되는지 확인해보시면 좋을 것 같아요!
private val _searchResults = MutableLiveData<List<Place>>() | ||
|
||
val searchResults: LiveData<List<Place>> get() = _searchResults |
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.
Backing field 에 대해서 잘 이해하고 계시군요! Coroutine 을 사용중이면, Flow 를 사용하여 처리해보는건 어떨까요?
- repository에 context를 직접적으로 넘겨주지 않게 변경 - 리사이클러 뷰에서 notifydatasetchanged() 사용하지 않도록 변경
코드 작성하면서 어려웠던 점
MVVM 패턴, data binding을 처음 사용해봐서 교수님, 구글링, gpt 등 많이 도움 받고 공부해서 작성했습니다.
하나의 액티비티에 리사이클러 뷰도 2개 사용하고 view model도 2개 사용했는데 fragment를 사용하면 코드가 더 좋아질지 궁금합니다.(+ 리사이클러 뷰 어댑터 클래스를 따로 파일을 만드는 게 나은 방식인지도 궁금합니다.)
중점적으로 리뷰해주셨으면 하는 부분
data binding을 더 사용해서 main activity의 코드를 더 줄일 수 있는 부분이 있는 지 궁금합니다.
리사이클러 뷰 구현이 전체적으로 어색하지 않은 지 궁금합니다.
저장된 검색어 목록에 데이터를 추가할 때 name이 중복된 경우를 생각해서 place의 id값을 참조해서 사용했는데 올바르게 사용했는지 궁금합니다.
결과 화면