-
Notifications
You must be signed in to change notification settings - Fork 0
[Feature] Logger 구현 #49
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
base: develop
Are you sure you want to change the base?
Conversation
b0696e5
to
c2d6f0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존 시스템 로그를 완벽히 대체하려는 것으로 보이는데,
그렇다면 object로 진입점을 뚫어줘서 어디서든 접근가능하게 하는 게 편할 거 같은데 어때
로거를 매번 주입받아 사용해야 하는 건 좀 불편한 거 같아서
override fun logAndToast(throwable: Throwable) { | ||
e(throwable) | ||
|
||
Toast.makeText( | ||
context, | ||
context.resources.getString(R.string.error_default_message), | ||
Toast.LENGTH_SHORT | ||
).show() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p5; 시스템 토스트 쓰려나 우리 커스텀 토스트 쓰는 거 같은데
@dudwls901 logger를 주입받아 사용하려는 이유는 UI 통합 테스트 시, Singleton으로 인해 강하게 커플링이 걸려 테스트를 방해할 수 있기 때문이야. 대신에 DI를 사용하면 목킹하여 쉽게 대처할 수 있어, 커플링을 느슨하게 줄일 수 있어. 커플링을 계속 줄이기 위해서는 싱글톤 패턴들은 조금 지양하는게 좋을 것 같아서 DI를 사용하도록했어. Logger.LogAndToast는 MainActivity에서 Logger를 주입해 모든 Root Composable의 Navigation에서 Logger.LogAndToast를 호출하는 람다를 통해 사용할 수 있도록 하면 좋을 것 같아. 반면 ViewModel에서 자주 inject해야할 것 같은데 이건 BaseViewModel 같은 것을 만들고 여기서 Logger를 주입해 사용하는 방법으로 한다면 매번 주입할 확률이 줄어드니 괜찮을 것 같은데 어때? |
두 가지가 있을 것 같은데 테스트 용이성은 현재 DI 방식은 유지하고 object로 진입점만 열어두는 것을 생각했음! object AppLogger : Logger {
private var delegate: Logger? = null
fun init(logger: Logger) {
delegate = logger
}
...
}
//Application
@Inject lateinit var logger: Logger
override fun onCreate() {
super.onCreate()
AppLogger.init(logger)
} |
좋네. 이렇게 하면 좀 더 유연하게 사용할 수 있겠어. 수정해놓을게. 대신에 웬만해서는 주입받아 써야하는게 약속이고 어쩔 수 없는 경우에 써야한다면 직접 접근하여 사용하는 것으로 이야기하면 좋을듯 |
'웬만해서는 주입받아 써야하는 게 약속'이라면 object 열어두지 말고 주입 받아서 사용만 하는 게 더 좋아보이긴해! 다만 아직 모르겠는 부분은 object를 열어두었음에도 왜 주입받아 써야하는지는 잘 모르겠어..! 그래서, 결론은 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흠 그럼 주입 받아서 쓰는 걸 원칙으로 하되, 예외 상황인 viewmodel이나 UI 그릴 때만 만들어둔 object를 쓰는 걸로 가는걸까요??
아니면 강휘오빠 말대로 주입받아서 쓰는게 기본 원칙이기 때문에 DI만 ..?
강휘오빠 넘 수고 많앗어용 .. 코드 보면서 또 공부 왕창 해가여.. 🥹
buildTypes { | ||
release { | ||
debug { | ||
applicationIdSuffix = ".dev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3: 오 .. 처음 봤어요
이렇게 하면 릴리즈 앱이랑 디버그 앱을 같은 기기에 동시에 설치할 수 잇군요 .. ! 배워갑니더
아니 웬만해서는 주입받아 써야하고 어쩔 수 없이 사용이 필요한 경우에만 쓰면 될 것 같아. 확장함수에서 사용이 필요하다던가.. 등등..? xml 기반일 때는 커스텀 뷰에서도 사용가능하고..근데 이러한 경우를 제외하곤 거의 직접적으로 object로 접근해서 쓰는 일은 없을 것 같네 |
변경사항 요약