Merge pull request #48 from boostcampwm-2022/modify/codereview

koreatlwls · web-flow · commit 3e34061c9297 · 2022-11-27T13:07:56.000+09:00
[PR] 멘토님 리뷰 받은 부분 수정
diff --git a/data/src/main/java/com/stop/data/di/DataSourceModule.kt b/data/src/main/java/com/stop/data/di/DataSourceModule.kt
@@ -32,6 +32,6 @@ internal interface DataSourceModule {
     @Singleton
     fun provideAlarmLocalDataSource(
         alarmLocalDataSourceImpl: AlarmLocalDataSourceImpl
-    ): AlarmLocalDataSource
-
+    ): AlarmLocalDataSourc
+    
 }
diff --git a/data/src/main/java/com/stop/data/di/RepositoryModule.kt b/data/src/main/java/com/stop/data/di/RepositoryModule.kt
@@ -24,9 +24,13 @@ internal interface RepositoryModule {
 
     @Binds
     @Singleton
-    fun provideNearPlaceRepository(nearPlaceRepositoryImpl: NearPlaceRepositoryImpl): NearPlaceRepository
+    fun provideNearPlaceRepository(
+        nearPlaceRepositoryImpl: NearPlaceRepositoryImpl
+    ): NearPlaceRepository
+
 
     @Binds
     @Singleton
     fun provideAlarmRepository(alarmRepositoryImpl: AlarmRepositoryImpl): AlarmRepository
+
 }
diff --git a/data/src/main/java/com/stop/data/remote/source/nearplace/NearPlaceRemoteDataSource.kt b/data/src/main/java/com/stop/data/remote/source/nearplace/NearPlaceRemoteDataSource.kt
@@ -5,11 +5,11 @@ import com.stop.data.model.nearplace.Place
 interface NearPlaceRemoteDataSource {
 
     suspend fun getNearPlaces(
-        version : Int,
+        version: Int,
         searchKeyword: String,
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ): List<Place>
+    ): Result<List<Place>>
 
 }
diff --git a/data/src/main/java/com/stop/data/remote/source/nearplace/NearPlaceRemoteDataSourceImpl.kt b/data/src/main/java/com/stop/data/remote/source/nearplace/NearPlaceRemoteDataSourceImpl.kt
@@ -15,29 +15,30 @@ internal class NearPlaceRemoteDataSourceImpl @Inject constructor(
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ): List<Place> {
+    ): Result<List<Place>> {
         val result = nearPlaceApiService.getNearPlaces(
             version,
             searchKeyword,
             centerLon,
             centerLat,
             appKey
         )
-
-        when (result) {
-            is NetworkResult.Failure -> {
-                throw Exception(result.message)
-            }
-            is NetworkResult.Success -> {
-                return result.data.searchPoiInfo.pois.poi.map {
-                    it.toRepositoryModel()
+        return runCatching {
+            when (result) {
+                is NetworkResult.Failure -> {
+                    throw Exception(result.message)
+                }
+                is NetworkResult.Success -> {
+                    result.data.searchPoiInfo.pois.poi.map {
+                        it.toRepositoryModel()
+                    }
+                }
+                is NetworkResult.NetworkError -> {
+                    throw result.exception
+                }
+                is NetworkResult.Unexpected -> {
+                    throw result.exception
                 }
-            }
-            is NetworkResult.NetworkError -> {
-                throw result.exception
-            }
-            is NetworkResult.Unexpected -> {
-                throw result.exception
             }
         }
     }
diff --git a/data/src/main/java/com/stop/data/repository/NearPlaceRepositoryImpl.kt b/data/src/main/java/com/stop/data/repository/NearPlaceRepositoryImpl.kt
@@ -3,8 +3,6 @@ package com.stop.data.repository
 import com.stop.data.remote.source.nearplace.NearPlaceRemoteDataSource
 import com.stop.domain.model.nearplace.Place
 import com.stop.domain.repository.NearPlaceRepository
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.flow
 import javax.inject.Inject
 
 internal class NearPlaceRepositoryImpl @Inject constructor(
@@ -17,18 +15,20 @@ internal class NearPlaceRepositoryImpl @Inject constructor(
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ): Flow<List<Place>> = flow {
-        emit(
-            nearPlaceRemoteDataSource.getNearPlaces(
-                version,
-                searchKeyword,
-                centerLon,
-                centerLat,
-                appKey
-            ).map {
+    ): List<Place> {
+        nearPlaceRemoteDataSource.getNearPlaces(
+            version,
+            searchKeyword,
+            centerLon,
+            centerLat,
+            appKey
+        ).onSuccess { places ->
+            return places.map {
                 it.toUseCaseModel()
             }
-        )
+        }.onFailure {
+            throw it
+        }
+        return emptyList()
     }
-
 }
diff --git a/domain/src/main/java/com/stop/domain/repository/NearPlaceRepository.kt b/domain/src/main/java/com/stop/domain/repository/NearPlaceRepository.kt
@@ -1,7 +1,6 @@
 package com.stop.domain.repository
 
 import com.stop.domain.model.nearplace.Place
-import kotlinx.coroutines.flow.Flow
 
 interface NearPlaceRepository {
 
@@ -11,6 +10,6 @@ interface NearPlaceRepository {
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ): Flow<List<Place>>
+    ): List<Place>
 
 }
diff --git a/domain/src/main/java/com/stop/domain/usecase/nearplace/GetNearPlacesUseCase.kt b/domain/src/main/java/com/stop/domain/usecase/nearplace/GetNearPlacesUseCase.kt
@@ -1,7 +1,6 @@
 package com.stop.domain.usecase.nearplace
 
 import com.stop.domain.model.nearplace.Place
-import kotlinx.coroutines.flow.Flow
 
 interface GetNearPlacesUseCase {
 
@@ -11,6 +10,6 @@ interface GetNearPlacesUseCase {
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ) : Flow<List<Place>>
+    ): List<Place>
 
 }
diff --git a/domain/src/main/java/com/stop/domain/usecase/nearplace/GetNearPlacesUseCaseImpl.kt b/domain/src/main/java/com/stop/domain/usecase/nearplace/GetNearPlacesUseCaseImpl.kt
@@ -2,7 +2,6 @@ package com.stop.domain.usecase.nearplace
 
 import com.stop.domain.model.nearplace.Place
 import com.stop.domain.repository.NearPlaceRepository
-import kotlinx.coroutines.flow.Flow
 import javax.inject.Inject
 
 internal class GetNearPlacesUseCaseImpl @Inject constructor(
@@ -15,7 +14,7 @@ internal class GetNearPlacesUseCaseImpl @Inject constructor(
         centerLon: Double,
         centerLat: Double,
         appKey: String
-    ): Flow<List<Place>> =
+    ): List<Place> =
         nearPlaceRepository.getNearPlaces(
             version,
             searchKeyword,
diff --git a/presentation/src/main/java/com/stop/ui/map/MapFragment.kt b/presentation/src/main/java/com/stop/ui/map/MapFragment.kt
@@ -256,25 +256,22 @@ class MapFragment : Fragment() {
     }
 
     private fun observeClickCurrentLocation() {
-        viewLifecycleOwner.lifecycleScope.launch {
-            launch {
-                placeSearchViewModel.clickCurrentLocation
-                    .flowWithLifecycle(viewLifecycleOwner.lifecycle)
-                    .collect {
-                        val currentLocation = placeSearchViewModel.currentLocation
-                        val currentTMapPoint = TMapPoint(currentLocation.latitude, currentLocation.longitude)
-
-                        tMapView.setCenterPoint(currentTMapPoint.latitude, currentTMapPoint.longitude)
-
-                        setPanel(currentTMapPoint)
-
-                        makeMarker(
-                            MARKER,
-                            R.drawable.ic_baseline_location_on_32,
-                            currentTMapPoint
-                        )
-                    }
-            }
+        lifecycleScope.launch {
+            placeSearchViewModel.clickCurrentLocation
+                .flowWithLifecycle(viewLifecycleOwner.lifecycle)
+                .collect {
+                    val currentLocation = placeSearchViewModel.currentLocation
+                    val currentTmapPoint = TMapPoint(currentLocation.latitude, currentLocation.longitude)
+
+                    tMapView.setCenterPoint(currentTmapPoint.latitude, currentTmapPoint.longitude)
+
+                    setPanel(currentTmapPoint)
+
+                    makeMarker(
+                        R.drawable.ic_baseline_location_on_32,
+                        currentTmapPoint
+                    )
+                }
         }
     }
 
diff --git a/presentation/src/main/java/com/stop/ui/placesearch/NearPlaceAdapter.kt b/presentation/src/main/java/com/stop/ui/placesearch/NearPlaceAdapter.kt
@@ -15,6 +15,7 @@ class NearPlaceAdapter : ListAdapter<Place, NearPlaceAdapter.ViewHolder>(diffUti
     class ViewHolder(private val binding: ItemNearPlaceBinding) : RecyclerView.ViewHolder(binding.root) {
         fun bind(place: Place) {
             binding.place = place
+            binding.executePendingBindings()
         }
     }
 
diff --git a/presentation/src/main/java/com/stop/ui/placesearch/PlaceSearchFragment.kt b/presentation/src/main/java/com/stop/ui/placesearch/PlaceSearchFragment.kt
@@ -17,6 +17,10 @@ import androidx.navigation.findNavController
 import com.stop.R
 import com.stop.databinding.FragmentPlaceSearchBinding
 import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.flow.debounce
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 
 @AndroidEntryPoint
@@ -47,6 +51,7 @@ class PlaceSearchFragment : Fragment() {
         buttonClick()
         listenEditTextChange()
         logErrorMessage()
+        observeSearchKeyword()
     }
 
     private fun initAdapter() {
@@ -107,19 +112,31 @@ class PlaceSearchFragment : Fragment() {
     }
 
     private fun logErrorMessage() {
-        viewLifecycleOwner.lifecycleScope.launch {
-            launch {
-                placeSearchViewModel.errorMessage
-                    .flowWithLifecycle(viewLifecycleOwner.lifecycle)
-                    .collect {
-                        if (it.isNotBlank()) {
-                            Log.e(PLACE_SEARCH_FRAGMENT, it)
-                        }
+        lifecycleScope.launch {
+
+            placeSearchViewModel.errorMessage
+                .flowWithLifecycle(viewLifecycleOwner.lifecycle)
+                .collect {
+                    if (it.isNotBlank()) {
+                        Log.e(PLACE_SEARCH_FRAGMENT, it)
                     }
-            }
+                }
         }
     }
 
+    @OptIn(FlowPreview::class)
+    private fun observeSearchKeyword(){
+        placeSearchViewModel.searchKeyword.debounce(100)
+            .onEach {
+                if(it.isBlank()){
+                    placeSearchViewModel.setNearPlaceListEmpty()
+                }else{
+                    placeSearchViewModel.getNearPlaces(it)
+                }
+            }
+            .launchIn(lifecycleScope)
+    }
+
     override fun onDestroyView() {
         _binding = null
 
diff --git a/presentation/src/main/java/com/stop/ui/placesearch/PlaceSearchViewModel.kt b/presentation/src/main/java/com/stop/ui/placesearch/PlaceSearchViewModel.kt
@@ -16,7 +16,8 @@ import com.stop.model.Event
 import com.stop.model.Location
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.channels.Channel
-import kotlinx.coroutines.flow.collectLatest
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.receiveAsFlow
 import kotlinx.coroutines.launch
 import javax.inject.Inject
@@ -29,10 +30,10 @@ class PlaceSearchViewModel @Inject constructor(
 
     var currentLocation = Location(0.0, 0.0)
 
-    var bookmarks = mutableListOf(EXAMPLE_BOOKMARK_1, EXAMPLE_BOOKMARK_2, EXAMPLE_BOOKMARK_3)
+    private val _nearPlaceList = MutableStateFlow<List<Place>>(emptyList())
+    val nearPlaceList: StateFlow<List<Place>> = _nearPlaceList
 
-    private val _nearPlaceList = MutableLiveData<List<Place>>()
-    val nearPlaceList: LiveData<List<Place>> = _nearPlaceList
+    var bookmarks = mutableListOf(EXAMPLE_BOOKMARK_1, EXAMPLE_BOOKMARK_2, EXAMPLE_BOOKMARK_3)
 
     private val errorMessageChannel = Channel<String>()
     val errorMessage = errorMessageChannel.receiveAsFlow()
@@ -43,41 +44,33 @@ class PlaceSearchViewModel @Inject constructor(
     private val clickCurrentLocationChannel = Channel<Boolean>()
     val clickCurrentLocation = clickCurrentLocationChannel.receiveAsFlow()
 
+    private val _searchKeyword = MutableStateFlow("")
+    val searchKeyword : StateFlow<String> = _searchKeyword
+    
     private val _geoLocation = MutableLiveData<GeoLocationInfo>()
     val geoLocation: LiveData<GeoLocationInfo> = _geoLocation
 
     private val _panelVisibility = MutableLiveData(View.GONE)
     val panelVisibility: LiveData<Int> = _panelVisibility
 
     fun afterTextChanged(s: Editable?) {
-        getNearPlaces(
-            s.toString(),
-            currentLocation.longitude,
-            currentLocation.latitude
-        )
-
-        if (s.toString().isBlank()) {
-            _nearPlaceList.postValue(emptyList())
-        }
+        _searchKeyword.value = s.toString()
     }
 
-    private fun getNearPlaces(
+    fun getNearPlaces(
         searchKeyword: String,
-        centerLon: Double,
-        centerLat: Double
     ) {
         viewModelScope.launch {
             try {
-                getNearPlacesUseCase.getNearPlaces(
-                    TMAP_VERSION,
-                    searchKeyword,
-                    centerLon,
-                    centerLat,
-                    BuildConfig.TMAP_APP_KEY
-                ).collectLatest {
-                    _nearPlaceList.postValue(it)
-                    Log.d("PlaceSearchViewModel","getNearPlace $it")
-                }
+                _nearPlaceList.emit(
+                    getNearPlacesUseCase.getNearPlaces(
+                        TMAP_VERSION,
+                        searchKeyword,
+                        currentLocation.longitude,
+                        currentLocation.latitude,
+                        BuildConfig.TMAP_APP_KEY
+                    )
+                )
             } catch (e: Exception) {
                 setNearPlaceListEmpty()
                 errorMessageChannel.send(e.message ?: "something is wrong")
@@ -87,7 +80,7 @@ class PlaceSearchViewModel @Inject constructor(
     }
 
     fun setNearPlaceListEmpty() {
-        _nearPlaceList.postValue(emptyList())
+        _nearPlaceList.value = emptyList()
     }
 
     fun setClickPlace(place: Place) {
@@ -114,5 +107,4 @@ class PlaceSearchViewModel @Inject constructor(
         private val EXAMPLE_BOOKMARK_3 = Location(37.48450549635376, 126.89324337770405)
     }
 
-
 }

Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,6 @@ internal interface DataSourceModule {`
`32`	`32`	`@Singleton`
`33`	`33`	`fun provideAlarmLocalDataSource(`
`34`	`34`	`alarmLocalDataSourceImpl: AlarmLocalDataSourceImpl`
`35`		`- ): AlarmLocalDataSource`
`36`		`-`
	`35`	`+ ): AlarmLocalDataSourc`
	`36`	`+`
`37`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ class NearPlaceAdapter : ListAdapter<Place, NearPlaceAdapter.ViewHolder>(diffUti`
`15`	`15`	`class ViewHolder(private val binding: ItemNearPlaceBinding) : RecyclerView.ViewHolder(binding.root) {`
`16`	`16`	`fun bind(place: Place) {`
`17`	`17`	`binding.place = place`
	`18`	`+ binding.executePendingBindings()`
`18`	`19`	`}`
`19`	`20`	`}`
`20`	`21`