HW-1 RxJava 네이버 영화정보 검색 어플리케이션 #4
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NackUn
changed the title
sport0102
requested changes
Dec 21, 2019
sport0102
left a comment
sport0102
left a comment
There was a problem hiding this comment.
수정 필요없다고 생각하시는 부분은 댓글 달아주시고, 수정하신 부분에는 커밋번호 넣어주세요!
|
|
||
| import com.example.navermoviesample.vo.MovieItem | ||
|
|
||
| class Repository private constructor( |
There was a problem hiding this comment.
Repository 이름이 명확했으면 좋겠습니다. 어떤 걸 다루는 Repository인지 명시해주세요
| class Repository private constructor( | ||
| private val remoteDataSource: DataSource | ||
| ) : DataSource { | ||
| override fun requestMovies( |
There was a problem hiding this comment.
우리가 RxJava를 배웠으니까 RxJava Single을 사용해서 가져오는 방식은 어떨까요?
NaverMovieSample/app/src/main/java/com/example/navermoviesample/data/Repository.kt
Outdated
Show resolved
Hide resolved
NaverMovieSample/app/src/main/java/com/example/navermoviesample/data/remote/RemoteDataSource.kt
Outdated
Show resolved
Hide resolved
NaverMovieSample/app/src/main/java/com/example/navermoviesample/ui/movie/MovieViewModel.kt
Show resolved
Hide resolved
NaverMovieSample/app/src/main/java/com/example/navermoviesample/ui/movie/MovieViewModel.kt
Outdated
Show resolved
Hide resolved
| val child = rv.findChildViewUnder(e.x, e.y) | ||
| val position = rv.getChildAdapterPosition(child!!) | ||
|
|
||
| val webIntent: Intent = Uri.parse(vm.movieItems.value!![position].link).let { webpage -> |
There was a problem hiding this comment.
null safety 처리를 위해서 ?.let이 아니라면 굳이 let을 사용할게 아니라 webpage라는 변수에 그냥 앞의 내용을 넣어도 될 것 같습니다.
| import androidx.databinding.BindingAdapter | ||
| import com.bumptech.glide.Glide | ||
|
|
||
| @BindingAdapter("setUrlImg") |
There was a problem hiding this comment.
바인딩 이름에 bind: 를 붙여주면 다른 함수랑 겹치지 않아서 헷갈리지 않더라구요!
Author
There was a problem hiding this comment.
좋은 의견 감사합니다. xml에 bind: 로 하면 xmlns을 추가해줘야되는 것 같네요. 조금 더 생각해보도록 하겠습니다!
Author
There was a problem hiding this comment.
해당 부분은 layout에 추가되는 것 때문에 bind: 로 안하는 게 제 생각에는 괜찮은 것 같습니다~
| type="com.example.navermoviesample.ui.movie.MovieViewModel" /> | ||
| </data> | ||
|
|
||
| <LinearLayout |
Author
sport0102
added
Answer Needed
NackUn
added
Review Needed
NackUn
pushed a commit
to NackUn/RxJavaStudy
that referenced
this pull request
Feb 8, 2020
namjackson (조남재) 의 공간입니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.