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

전남대 Android_신혜서 5주차 과제 수정(Step2) #75

Open
wants to merge 64 commits into
base: tlsgptj
Choose a base branch
from

Conversation

tlsgptj
Copy link

@tlsgptj tlsgptj commented Jul 26, 2024

기능 요구 사항

  1. MVVM 아키텍처 패턴을 적용한다.
  2. DataBinding, LiveData를 사용한다.

비동기 처리를 Coroutine으로 변경한다.

  1. 프로그래밍 요구 사항
  2. 코드 컨벤션을 준수하며 프로그래밍한다.

소감

LiveData와 Databinding에 대해서 다뤄보지 않아 아직 미숙한 상태입니다..
MVVM아키텍쳐로 나름대로 분리해보았지만 잘못 분리된 부분은 수정하도록 하겠습니다..!

@tlsgptj tlsgptj changed the title 전남대 Android_신혜서 (Step2) 전남대 Android_신혜서 5주차 과제 (Step2) Jul 26, 2024
Copy link

@bigstark bigstark left a comment

Choose a reason for hiding this comment

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

혜서님 5주차도 고생 많으셨습니다.


전반적으로 이전의 피드백이 반영이 되지 않은 듯 합니다. 가장 기본적으로 코드 스타일이 맞지 않아요. class 나 package 의 네이밍 컨벤션부터 신경써주시면 좋을 것 같습니다. 또한 hilt 의 적용이 제대로 되어있지 않은 것 같아요. Application 이 제대로 설정되어있지 않다면 hilt graph 도 제대로 그려지지 않을 거에요. 또한 module 은 존재하지만 주입받아서 사용하는 곳이 없는데, 그부분은 꼭 수정해주셨으면 좋겠습니다.

import androidx.recyclerview.widget.RecyclerView
import campus.tech.kakao.map.R

class SavedSearchAdapter(

Choose a reason for hiding this comment

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

여기도 ListAdapter 로 변경해주세요!

Comment on lines 32 to 40
class PlaceViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {
private val nameTextView: TextView = itemView.findViewById(R.id.name)
private val addressTextView: TextView = itemView.findViewById(R.id.place)

fun bind(place: Place) {
nameTextView.text = place.place_name
addressTextView.text = place.address_name
}
}

Choose a reason for hiding this comment

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

viewbinding 을 이용하시는 것은 어떨까요?

notifyDataSetChanged()
}

inner class SavedSearchViewHolder(itemView: View, private val onItemClick: (String) -> Unit) :

Choose a reason for hiding this comment

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

여기도 마찬가지로 inner class 를 사용하지 않는 방향으로 수정해주세요.

Comment on lines 40 to 50
itemView.setOnClickListener {
onItemClick(searchText)
}
deleteButton.setOnClickListener {
val position = adapterPosition
if (position != RecyclerView.NO_POSITION) {
val updatedData = data.toMutableList()
updatedData.removeAt(position)
updateData(updatedData.toString())
}
}

Choose a reason for hiding this comment

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

bind 시점에 click listener 설정하지 않도록 해주세요.

Comment on lines +14 to +43
constructor(parcel: Parcel) : this(
parcel.readString()!!,
parcel.readString()!!,
parcel.readString()!!,
parcel.readDouble(),
parcel.readDouble()
)

override fun writeToParcel(parcel: Parcel, flags: Int) {
parcel.writeString(place_name)
parcel.writeString(address_name)
parcel.writeString(category_group_name)
parcel.writeDouble(latitude)
parcel.writeDouble(longitude)
}

override fun describeContents(): Int {
return 0
}

companion object CREATOR : Parcelable.Creator<Place> {
override fun createFromParcel(parcel: Parcel): Place {
return Place(parcel)
}

override fun newArray(size: Int): Array<Place?> {
return arrayOfNulls(size)
}
}
}

Choose a reason for hiding this comment

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

kotlin-parcelize 를 사용하신다면 생성하지 않아도 됩니다.

Comment on lines 21 to 28
private val _searchQuery = MutableLiveData<String?>()
val searchQuery: LiveData<String?> get() = _searchQuery
private val _isSavedSearchesVisible = MutableLiveData<Boolean>()
val isSavedSearchesVisible: LiveData<Boolean> get() = _isSavedSearchesVisible
private val _searchResults = MutableLiveData<List<SearchResult>?>()
val searchResults: LiveData<List<SearchResult>?> get() = _searchResults
private var _savedSearches = MutableLiveData<List<Place>>()
val savedSearches: LiveData<List<Place>> get() = _savedSearches

Choose a reason for hiding this comment

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

LiveData 사용 시 nullable 처리를 굳이 하지 않으셔도 됩니다.

Copy link
Author

Choose a reason for hiding this comment

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

넵 수정하겠습니다!

Comment on lines 29 to 30
val savedSearchAdapter = SavedSearchAdapter()
val searchResultAdapter = PlaceAdapter()

Choose a reason for hiding this comment

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

adapter 는 ViewModel 이 아닌 Activity 와 같은 View 에서 처리해주세요

Copy link
Author

Choose a reason for hiding this comment

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

넵!

@Provides
@Singleton
fun providePlacesClient(@ApplicationContext context: Context): PlacesClient {
Places.initialize(context, "AIzaSyCUncz7v8nwT3m5OHasVJTep1e1549yAKM")

Choose a reason for hiding this comment

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

api key 는 build config 로 이동해주세요.

Copy link
Author

Choose a reason for hiding this comment

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

인터넷에서 찾아봤는데 def로 선언하고 api키를 집어넣더라고요..근데 따라하니까 def쪽이 사용할 수 없는 함수라고 떠서 다른 자료들을 찾아보고 있는 중입니당...

Comment on lines 7 to 27
@Database(entities = [SearchResult::class], version = 1)
abstract class AppDatabase : RoomDatabase() {
abstract fun searchDao(): SearchDao

companion object {
@Volatile
private var INSTANCE: AppDatabase? = null

fun getDatabase(context: Context): AppDatabase {
return INSTANCE ?: synchronized(this) {
val instance = Room.databaseBuilder(
context.applicationContext,
AppDatabase::class.java,
"History.db"
).build()
INSTANCE = instance
instance
}
}
}
}

Choose a reason for hiding this comment

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

AppDatabase 는 어디에서 주입해주고 어디에서 주입받고 있나요?

import dagger.hilt.android.HiltAndroidApp

@HiltAndroidApp
class myApplication : Application()

Choose a reason for hiding this comment

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

AndroidManifest 에 추가하지 않은 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

manifest 파일에 myApplication 클래스를 추가하는 것이 맞나요?

@tlsgptj
Copy link
Author

tlsgptj commented Jul 30, 2024

일단 수정하긴 했는데 혹시
image
이 오류랑
image
여기에서 형 변환 오류가 발생합니다...저번주부터 이것저것 넣어봤는데 자꾸 오류가 발생해서 뭘 어떻게 구현해야할 지 모르겠습니다......ㅜ

@tlsgptj tlsgptj changed the title 전남대 Android_신혜서 5주차 과제 (Step2) 전남대 Android_신혜서 5주차 과제 수정(Step2) Jul 30, 2024
private fun loadSavedSearches() {
viewModelScope.launch {
val searchResults = searchRepository.getSearchResults(savedSearches.toString())
//_savedSearches.value = searchResults
Copy link

Choose a reason for hiding this comment

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

_savedSearches 는 List 타입이고, searchResults 는 List 타입이에요. 차근차근 에러를 파악해보시면 좋을 것 같습니다.

Comment on lines +7 to +16
@InverseMethod("stringToSearchQuery")
@JvmStatic
fun searchQueryToString(value: String): String {
return value
}

@JvmStatic
fun stringToSearchQuery(value: String): String {
return value
}
Copy link

Choose a reason for hiding this comment

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

받은 string 을 그대로 반환하는 것이라 굳이 필요없을 것 같습니다.

import campus.tech.kakao.map.Data.Place
import campus.tech.kakao.map.R

class SavedSearchAdapter : RecyclerView.Adapter<SavedSearchAdapter.SavedSearchViewHolder>() {
Copy link

Choose a reason for hiding this comment

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

이부분도 ListAdapter 를 한번 활용해보심이 좋을 것 같네요


@HiltViewModel
class SearchViewModel @Inject constructor(
private val searchRepository: SearchRepository
Copy link

Choose a reason for hiding this comment

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

searchRepository 를 주입해주는 곳은 어디죠?

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.

None yet

3 participants