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_노신 3주차 과제 Step2 #81

Open
wants to merge 30 commits into
base: normenghub
Choose a base branch
from

Conversation

Normenghub
Copy link

@Normenghub Normenghub commented Jul 11, 2024

어려웠던 점, 느낀 점

mvvm모델 아직 적용 못했습니다.. 다음주 과제 스텝 0때 적용시키고 pr올리도록 노력해보겠습니다.

일단 구현을 완료 시키고 프래그먼트로 리팩토링했는데 코드가 깔끔해진것 같습니다. 적용해보니 많이 배우는것 같습니다.!

중점적으로 리뷰해주셨으면 하는 부분

서치 프래그먼트 불러올때 키보드가 내려가는데 이걸 어떻게 해결해야하는지 모르겠습니다...

@Normenghub Normenghub changed the title Step2 부산대 Android_노신 3주차 과제 Step2 Jul 11, 2024
@LeeOhHyung LeeOhHyung self-requested a review July 14, 2024 16:05
Copy link

@LeeOhHyung LeeOhHyung left a comment

Choose a reason for hiding this comment

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

@Normenghub 점점 발전하고 있으신게 느껴집니다.
3주차 과제도 고생많으셨구요. 수정사항 반영해서 PR 올리신다면 다시 확인해보도록 할게요!

Comment on lines +19 to +20
buildConfigField("String", "KAKAO_REST_API_KEY", "\"${getApiKey("KAKAO_REST_API_KEY")}\"")
resValue("string", "kakao_api_key", getApiKey("kakao_api_key"))

Choose a reason for hiding this comment

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

buildConfigField, resValue 둘다 local.properties 에 있는 문자열을 가져와서 쓸려고 하는것인데, 다르게 사용하신 이유가 있었나요? 둘다 사용할 수 있습니다만, 다른 방식으로 문자열 가지고 오고 있어서 궁금했습니다.

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 48 to 49
finish()
super.onBackPressed()

Choose a reason for hiding this comment

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

super.onBackPressed() 호출해도 finish() 까지 호출될것이라 super.onBackPressed() 만 호출해도 괜찮을것 같아요!

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 +9 to +12
<FrameLayout
android:id="@+id/fragmentContainer"
android:layout_width="match_parent"
android:layout_height="match_parent"/>

Choose a reason for hiding this comment

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

fragment를 사용할때, container로 사용하는 FrameLayout에 commit/add/replace해주는것이 일반적으로 많이 사용하는것은 맞습니다. 최근에는 FragmentContainerView 라는 위젯도 사용하고 있습니다. 동일한 상황에서 쓸수있는데 전환 애니메이션 처리를 자동으로 해준다는것 정도의 차이가 있습니다. 여유가 되신다면 한번 사용해보세요.

(참고) https://developer.android.com/reference/androidx/fragment/app/FragmentContainerView

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 +16 to +27
<androidx.appcompat.widget.SearchView
android:id="@+id/searchView"
android:layout_width="match_parent"
android:layout_height="70dp"
android:layout_margin="10dp"
app:iconifiedByDefault="false"
android:background="@color/white"
app:queryHint="검색어를 입력해 주세요."
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent"
/>

Choose a reason for hiding this comment

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

searchView 가 fragment_map 에도 있고, fragment_search 에도 있네요.
fragment를 사용한다면, container 내에서 교체한 해서 사용하고, 검색창은 공통으로 사용하는것이니 activity_main에서 가지고 있도록 하는게 어떨까 싶습니다. 화면 구조를 searchView 아래에 FrameLayout이 있는 구조가 되는것이죠!

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 52 to 66
private fun isNetworkAvailable(context: Context): Boolean {
val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
val network = connectivityManager.activeNetwork ?: return false
val activeNetwork = connectivityManager.getNetworkCapabilities(network) ?: return false
return when {
activeNetwork.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> true
activeNetwork.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> true
else -> false
}
} else {
val networkInfo = connectivityManager.activeNetworkInfo
return networkInfo != null && networkInfo.isConnected
}
}

Choose a reason for hiding this comment

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

Build.VERSION.SDK_INT >= Build.VERSION_CODES.M 이 부분은 현재 최소지원버전이 마시멜로우 이상이기 때문에 제거해도 괜찮을것 같습니다.

네트워크 연결 체크까지 구현하신것은 매우 좋습니다.
다양한 상황을 예외처리 한다면 앱의 완성도가 높아지기 때문이죠. 이것을 고려해야겠다는 것을 떠올리신것만으로도 좋네요

Copy link
Author

Choose a reason for hiding this comment

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

확인했습니다. 수정하겠습니다!

@Normenghub Normenghub requested a review from LeeOhHyung July 15, 2024 06:34
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