log: đặt log khi sign up #136
Conversation
WalkthroughThe pull request introduces comprehensive logging functionality to the Changes
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/pwhs/quickmem/presentation/auth/signup/email/SignupWithEmailViewModel.kt (1)
Line range hint
186-191: Loại bỏ logic kiểm tra email bị lặp lạiHiện tại có 2 nơi kiểm tra email validity:
- Trong
onEventkhi email thay đổi- Trong
validateInput()Nên tách logic kiểm tra email thành một method riêng để tái sử dụng.
+ private fun validateEmail(email: String): Boolean { + return email.emailIsValid() + } fun onEvent(event: SignUpWithEmailUiAction) { when (event) { is SignUpWithEmailUiAction.EmailChanged -> { - if (!event.email.emailIsValid()) { + if (!validateEmail(event.email)) { _uiState.update { it.copy( email = event.email, emailError = R.string.txt_invalid_email ) } } } } } private fun validateInput(): Boolean { var isValid = true - if (!uiState.value.email.validEmail() || uiState.value.email.isEmpty()) { + if (!validateEmail(uiState.value.email)) { _uiState.update { it.copy(emailError = R.string.txt_invalid_email_address) } isValid = false }Also applies to: 64-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/pwhs/quickmem/presentation/auth/signup/email/SignupWithEmailViewModel.kt(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/src/main/java/com/pwhs/quickmem/presentation/auth/signup/email/SignupWithEmailViewModel.kt (1)
Pattern **/*.kt: Thân thiện, chuyên nghiệp: Các nhận xét cần khách quan, lịch sự, tránh giọng điệu chỉ trích. Đề xuất cách cải thiện nếu thấy cần thiết, thay vì chỉ ra lỗi mà không có hướng dẫn.
Trọng tâm và hiệu quả: Nêu rõ ràng vấn đề chính, không đi quá chi tiết nếu không cần thiết, và đưa ra ví dụ cụ thể khi thảo luận về các phần cần thay đổi. Ví dụ, nếu có lỗi về logic, hãy chỉ ra phần mã cụ thể và đề xuất cách khắc phục.
Khuyến khích và ghi nhận nỗ lực: Khi có đóng góp tốt hoặc phát hiện một tính năng mới có giá trị, hãy công nhận đóng góp đó. Điều này giúp động viên các thành viên và tạo môi trường làm việc tích cực.
Sử dụng ngôn ngữ dễ hiểu: Cố gắng tránh các thuật ngữ quá kỹ thuật hoặc không cần thiết. Nếu sử dụng các thuật ngữ phức tạp, hãy đảm bảo giải thích hoặc cung cấp thêm thông tin để đảm bảo sự nhất quán và dễ hiểu cho tất cả thành viên.
Chú trọng vào cải tiến và tính bền vững: Bất cứ thay đổi nào được đề xuất đều nên chú trọng đến khả năng mở rộng và duy trì mã nguồn, cũng như ảnh hưởng đến các phần khác của hệ thống.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
| @@ -1,5 +1,6 @@ | |||
| package com.pwhs.quickmem.presentation.auth.signup.email | |||
|
|
|||
| import android.util.Log | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cải thiện cách sử dụng logging
Có 2 vấn đề cần xem xét:
- Đang import và sử dụng cả 2 thư viện logging (android.util.Log và Timber) là không cần thiết
- Tag nên được định nghĩa là constant ở cấp độ class thay vì local variable
-import android.util.Log
import timber.log.Timber
@HiltViewModel
class SignupWithEmailViewModel @Inject constructor(
private val authRepository: AuthRepository,
) : ViewModel() {
+ companion object {
+ private const val TAG = "SignupWithEmailViewModel"
+ }
- val tag = "SignupWithEmailViewModel" // Remove thisSau đó thay thế tất cả Log.d/e(tag,...) bằng Timber.d/e(...)
Also applies to: 93-93
|
|
||
| is Resources.Loading -> { | ||
| // Do nothing | ||
| Log.d(tag, "Loading: Signing up") | ||
| } | ||
|
|
There was a problem hiding this comment.
Thiếu cập nhật trạng thái loading
Trong trường hợp Resources.Loading của signup process, chỉ có logging mà không cập nhật isLoading state.
is Resources.Loading -> {
- // Do nothing
Log.d(tag, "Loading: Signing up")
+ _uiState.update { it.copy(isLoading = true) }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| is Resources.Loading -> { | |
| // Do nothing | |
| Log.d(tag, "Loading: Signing up") | |
| } | |
| is Resources.Loading -> { | |
| Log.d(tag, "Loading: Signing up") | |
| _uiState.update { it.copy(isLoading = true) } | |
| } |
@andaupro e mới đặt log r đó, a review giúp e nha
Summary by CodeRabbit