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

Feat1/week8 #13

Open
wants to merge 42 commits into
base: develop
Choose a base branch
from
Open

Feat1/week8 #13

wants to merge 42 commits into from

Conversation

crownjoe
Copy link
Contributor

@crownjoe crownjoe commented Jan 1, 2024

📌𝘐𝘴𝘴𝘶𝘦𝘴

📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯

  • 클린 아키텍쳐 적용
  • 힐트 적용

💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴

독감이슈로 이제야 끝냈네요................... 근데 이거 진짜 완전 어렵군요
아직 긴가민가한 것 같아서 조금 더 공부해야할 것 같아요
아무튼 고잉 파이팅😎

@crownjoe crownjoe added the ⭐ [FEAT] 새로운 기능 구현 label Jan 1, 2024
Copy link
Member

@Marchbreeze Marchbreeze left a comment

Choose a reason for hiding this comment

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

정말...꽤나...야무진데요? 조금 감동 받았을지도
독감때문에 많이 고생했을텐데 ㅜ 수고 많았슴니당 !!!

Comment on lines +5 to +6
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
Copy link
Member

Choose a reason for hiding this comment

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

요 친구들이 추가된 이유는 뭘까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3주차 과제에 플로팅 버튼에 캡쳐 버튼 넣었었는데 이 때 쓰려고 넣었습니다!_!

android:exported="false">
</activity>
<activity android:name=".LoginActivity" android:exported="true">
<activity android:name=".presentation.login.LoginActivity" android:exported="true">
Copy link
Member

Choose a reason for hiding this comment

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

줄바꿈~~해주세요

Comment on lines +12 to +13
override suspend fun getUser(page: Int): ResponseUserDto =
UserService.getUserList(page)
Copy link
Member

Choose a reason for hiding this comment

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

코틀린스러운 문법 좋네요 !!!

Comment on lines 42 to 48
fun toUserEntity(): List<UserEntity> = data.map {
UserEntity(
first_name = it.first_name,
email = it.email,
avatar = it.avatar
)
}
Copy link
Member

Choose a reason for hiding this comment

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

굿굿~~~~

Comment on lines 11 to 14
@POST("api/v1/members")
fun signup(
@Body request: RequestSignupDto,
): Call<Unit>
Copy link
Member

Choose a reason for hiding this comment

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

서비스에서 서버통신을 진행하는 함수의 경우, 함수명 앞에 postSignUp, getFollowerList 와 같이 서버통신 형태를 함꼐 명시해주는 것이 권장됩니다 ~

Comment on lines 43 to 48
private fun signup() {
binding.signupButton.setOnClickListener {
val intent = Intent(this, SignUpActivity::class.java)
startActivity(intent)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

함수 네이밍을 맞춰봅시당 ~ 고잉 컨벤션을 따르면 initSignUpBtnListener 일 것 같아요.
인텐트 사용하는 과정에서도 객체 생성 없이 스코프 함수로 넘어갈 수 있어보이는데, 고잉 디테일 맞추기 문서 확인해보세요 !


private fun observeLoginResult() {
lifecycleScope.launch {
loginViewModel.uiState.collect { loginState ->
Copy link
Member

Choose a reason for hiding this comment

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

.collect 보다는 flowWithLifecycle을 사용하고, 왜 그래야한느지에 대해서 한번 찾아봅시다!
힌트는 flow와 liveData의 생명주기 관련 차이에 있어요 ㅎㅎ

}

private fun getUser(userList: List<UserEntity>) {
val adapter = binding.rvUser.adapter as? UserAdapter
Copy link
Member

Choose a reason for hiding this comment

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

어댑터가 이미 위의 connectAdapter에서 생성이 되었으니, adapter를 전역변수로 둔다면 지금처럼 binding에서 adapter를 다시 가져올 일이 없겠죠?

app:layout_constraintTop_toBottomOf="@id/signTextId">

<com.google.android.material.textfield.TextInputEditText
android:id="@+id/editTextId"
Copy link
Member

Choose a reason for hiding this comment

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

xml 네이밍 컨벤션 통일시켜주셍요~

Comment on lines +17 to +18
tools:listitem="@layout/item_user"
app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"/>
Copy link
Member

Choose a reason for hiding this comment

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

요런 부분도 보통 tools가 마지막에 위치하는 순서가 맞는데, 방금 다시 톡방에 공유해준 ctrl+s 활용하시면 자동으로 재배열될거에요 ~~

Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

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

몸도 많이 안좋았는데 너무 열심히 잘해줬는걸요!!!!
코드도 좋고 고생했습니다 :)
건강 앱잼~~💪

Comment on lines 7 to 50
@Serializable
data class ResponseUserDto(
@SerialName("page")
val page: Int = 0,
@SerialName("per_page")
val per_page: Int = 0,
@SerialName("total")
val total: Int = 0,
@SerialName("total_pages")
val total_pages: Int = 0,
@SerialName("data")
val data: List<ResponseUserData>,
@SerialName("support")
val support: Support,
) {
@Serializable
data class ResponseUserData(
@SerialName("id")
val id: Int = 0,
@SerialName("email")
val email: String = "",
@SerialName("first_name")
val first_name: String = "",
@SerialName("last_name")
val last_name: String = "",
@SerialName("avatar")
val avatar: String = ""
)
@Serializable
data class Support(
@SerialName("url")
val url: String = "",
@SerialName("text")
val text: String = ""
)
fun toUserEntity(): List<UserEntity> = data.map {
UserEntity(
first_name = it.first_name,
email = it.email,
avatar = it.avatar
)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

초기값 설정은 필요한 경우가 아니면 안해도 될 것 같습니다!

Comment on lines 18 to 19
private val View_Myprofile = 0
private val View_Friend = 1
Copy link
Member

Choose a reason for hiding this comment

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

이런 0, 1 과 같이 설정해두는 값은 companion object에 넣어두는게 좋을 것 같아용

Comment on lines +20 to +24
binding = ActivityLoginBinding.inflate(layoutInflater)
setContentView(binding.root)

binding.lifecycleOwner = this
binding.loginViewModel = loginViewModel
Copy link
Member

Choose a reason for hiding this comment

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

지금 프로젝트에서는 BaseActivity를 써서 위 내용들을 직접하지 않아도 되지만, 이 외에 다른 내용들도 함수화해서 onCreate내부는 함수 빼고는 깔끔하게 비워두기~

Comment on lines 43 to 48
private fun signup() {
binding.signupButton.setOnClickListener {
val intent = Intent(this, SignUpActivity::class.java)
startActivity(intent)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

intent 변수를 생성하기 보다는 스코프 함수로 해보는건 어떨까용

Comment on lines 20 to 36
try {
val response = authService.login(RequestLoginDto(id, password)).execute()

if (response.isSuccessful) {
val responseBody = response.body()
if (responseBody != null) {
_uiState.value = UiState.Success(responseBody)
} else {
_uiState.value = UiState.Error
}
} else {
_uiState.value = UiState.Error
}
} catch (e: Exception) {
Log.d("LoginViewModel", "실패")
_uiState.value = UiState.Error
}
Copy link
Member

Choose a reason for hiding this comment

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

지금도 충분히 좋지만, runCatching으로 바꿔봐도 좋을 것 같아요!


clicksignbtn()
observeSignupResult()
//observeSignupValid()
Copy link
Member

Choose a reason for hiding this comment

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

나중에 프로젝트 코드에는 꼭 필요한 주석을 제외하고는 다 지우고 올리기~

startActivity(loginIntent)
finish()
} else {
Toast.makeText(this@SignUpActivity, "회원가입 실패요", Toast.LENGTH_SHORT).show()
Copy link
Member

Choose a reason for hiding this comment

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

string 분리 합시당

Comment on lines +9 to +30
class UserAdapter : RecyclerView.Adapter<UserViewHolder>() {
private val userList = mutableListOf<UserEntity>()

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): UserViewHolder {
val binding = ItemUserBinding.inflate(LayoutInflater.from(parent.context), parent, false)
return UserViewHolder(binding)
}

override fun onBindViewHolder(holder: UserViewHolder, position: Int) {
holder.onBind(userList[position])
}

override fun getItemCount(): Int {
return userList.size
}

fun setUserList(userData: List<UserEntity>) {
userList.clear()
userList.addAll(userData)
notifyDataSetChanged()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

중간 시간이 된다면 ListAdapter를 공부해보고 적용하는 것도 좋을 것 같아요


class HomeActivity : AppCompatActivity() {
private lateinit var binding: ActivityHomeBinding
private var openFAB = false
Copy link
Member

Choose a reason for hiding this comment

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

이 값은 viewModel로 옮기고 observe 하면 더욱 좋을 것 같네용

Copy link
Member

@leeeyubin leeeyubin left a comment

Choose a reason for hiding this comment

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

아닛 너무 잘하는데요!!! 아픈데도 과제하느라 수고 많았어요!!

Comment on lines 33 to 36
} catch (e: Exception) {
Log.d("LoginViewModel", "실패")
_uiState.value = UiState.Error
}
Copy link
Member

Choose a reason for hiding this comment

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

오호 예외처리를 해줬군요 좋네요!!!

Comment on lines 39 to 46








Copy link
Member

Choose a reason for hiding this comment

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

불필요한 공백은 지워주면 좋을 것 같아용

Comment on lines 25 to 33
.onSuccess { userList ->
val User = userList.map { userEntity ->
UserEntity(
avatar = userEntity.avatar,
email = userEntity.email,
first_name = userEntity.first_name,
)
}
_userState.value = UiState.Success(User)
Copy link
Member

Choose a reason for hiding this comment

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

활용 좋습니다!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐ [FEAT] 새로운 기능 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants