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주차 과제_step1 #62

Open
wants to merge 7 commits into
base: yb0x00
Choose a base branch
from

Conversation

yb0x00
Copy link

@yb0x00 yb0x00 commented Jul 26, 2024

코드 작성하면서 어려웠던 점

  • Room, Hilt를 처음 사용해 시행착오가 있었습니다
  • Hilt 사용이 어려워 과제 제출이 계속 늦어졌습니다. 우선 Room만 적용하여 PR 합니다. step2에 이어서 Hilt를 적용하고자 합니다.

멘토님이 중점적으로 리뷰해 주셨으면 하는 부분

  • Room을 올바르게 사용하고 있는지 확인 부탁드립니다.

과제 제출 프로세스를 안내받았음에도 이렇게 매번 PR이 늦어 무척 죄송한 마음입니다. 정말 죄송합니다!


class DataSearchActivity : AppCompatActivity(), RecentAdapterListener, SearchAdapterListener {
private lateinit var searchViewModel: SearchViewModel
private lateinit var recentViewModel: RecentViewModel
private lateinit var recentViewModel: DBViewModel

Choose a reason for hiding this comment

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

변수 이름과 타입의 매칭이 잘되어있지 않은 것 같아요. 적절하게 네이밍을 변경해보세요.

Copy link
Author

Choose a reason for hiding this comment

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

238a119 반영했습니다:)
피드백 감사합니다!

@@ -50,7 +52,9 @@ class DataSearchActivity : AppCompatActivity(), RecentAdapterListener, SearchAda

//ViewModel 생성
searchViewModel = ViewModelProvider(this)[SearchViewModel::class.java]
recentViewModel = ViewModelProvider(this)[RecentViewModel::class.java]
dbRepo = SearchHistoryRepository(this)
recentViewModel =

Choose a reason for hiding this comment

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

Hilt를 통한 의존성 주입이 적용되어있지 않네요. Hilt를 통해 각 객체가 생성되어 뷰모델이 만들어지도록 변경해보세요.

Copy link
Author

Choose a reason for hiding this comment

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

518a06c 에서 변경하였습니다:)
피드백 감사합니다!

import campus.tech.kakao.map.dataRepository.SearchHistoryRepository
import kotlinx.coroutines.launch

class DBViewModel(private val searchHistoryRepo: SearchHistoryRepository) : ViewModel() {

Choose a reason for hiding this comment

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

DBViewModel이라는 이름 대신 뷰의 추상화 개념이 보여지는 이름으로 변경해보세요.

Copy link
Author

Choose a reason for hiding this comment

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

787f37b 반영했습니다:)
피드백 감사합니다!

@omjoonkim
Copy link

안녕하세요 영빈님~!. 과제의 요구사항인 Hilt적용이 거의 되어있지 않은 것 같아요. 적용하신 뒤에 다시 리뷰요청 해주세요.

@yb0x00
Copy link
Author

yb0x00 commented Jul 29, 2024

@omjoonkim
코치님, 제가 의존성 주입 부분만 수정을 했어야 했는데
착각하고
518a06c ,
16a8421 에서
Data Binding 부분까지 수정을 해버렸습니다.
죄송합니다!

Hilt 적용하고 step2 PR에도 반영하였습니다

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.

2 participants