4
\$\begingroup\$

I am an Android developer with only 5 months of experience. I am still learning and trying to do my best.

Right now I am interested in concept of Separation of concerns. I do understand what it means and I am trying my best to follow it in my android development, however, I found myself a bit confused when I started learning about Clean Architecture.

I would like to show my bit of code, and if, possible, get some feedback on my implementation and its correctness when it comes to Separation of concerns

Code [Note: I am using MVVM in this app]:

PopularMoviesFragment:

class MoviesPopularFragment : Fragment() {

    private val moviesViewModel: MoviesViewModel by activityViewModels()
    private val adapter = PopularMoviesRecyclerAdapter()

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        val binding = FragmentMoviesPopularBinding.inflate(inflater, container, false)

        binding.viewModel = moviesViewModel
        binding.popularMoviesRV.layoutManager = GridLayoutManager(requireContext(), 2)
        binding.popularMoviesRV.adapter = adapter

        moviesViewModel.popularMoviesState.observe(viewLifecycleOwner, { state ->
            when (state) {
                is PopularMoviesState.Content -> {
                    // TODO: Find a way to deal with this via DataBinding
                    binding.popularMoviesSwipeRefresh.isRefreshing = false
                    binding.progressBarLayout.visibility = View.GONE
                    binding.pageNavigation.visibility = View.VISIBLE

                    adapter.submitList(state.response.results)
                }
                is PopularMoviesState.Failure -> {
                    // TODO: Find a way to deal with this via DataBinding
                    binding.popularMoviesSwipeRefresh.isRefreshing = false
                    binding.progressBarLayout.visibility = View.GONE
                    binding.pageNavigation.visibility = View.VISIBLE

                    adapter.submitList(state.cachedListOfMovies)
                    showSnackbar(requireView(), getString(R.string.movies_popular_message_viewing_cached_content), Snackbar.LENGTH_INDEFINITE)
                }
            }
        })

        // TODO: Look into implementing this via DataBinding
        binding.popularMoviesSwipeRefresh.setOnRefreshListener {
            binding.popularMoviesSwipeRefresh.isRefreshing = true
            moviesViewModel.fetchPopularMovies()
        }

        return binding.root
    }

    private fun showSnackbar(view: View, message: String, length: Int) {
        val snackbar = Snackbar.make(view, message, length)
        snackbar.animationMode = Snackbar.ANIMATION_MODE_SLIDE
        snackbar.setAction(view.context.getString(R.string.alert_dialog_default_button_ok)) { snackbar.dismiss() }
        snackbar.show()
    }
}

MoviesViewModel.kt:

fun fetchPopularMovies(page: Int = currentPage) {
        viewModelScope.launch {
            val result = MainRepository.fetchPopularMovies(page = page)

            _popularMoviesState.value = when (result) {

                is PopularMoviesState.Loading -> PopularMoviesState.Loading
                is PopularMoviesState.Content -> {
                    //TODO: Perhaps find a way to work with pages in a better, more isolated way.
                    currentPage = result.response.page
                    PopularMoviesState.Content(result.response)
                }
                is PopularMoviesState.Failure -> {
                    PopularMoviesState.Failure(result.errorMessage, result.cachedListOfMovies)
                }
            }
        }
    }

MainRepository.kt

suspend fun fetchPopularMovies(page: Int = 1) = try {
        val response = IPopularMovies.getInstance().getMovies(page = page)

        if (response.code() == 200) {
            savePopularMoviesToCache(response.body()!!.results)
            PopularMoviesState.Content(response.body()!!)
        } else {
            PopularMoviesState.Failure(response.body()!!.status_message, popularMovieDao.getMovies() ?: listOf())
        }
    } catch (error: Throwable) {
        PopularMoviesState.Failure(error.localizedMessage!!, popularMovieDao.getMovies() ?: listOf())
    }

    private suspend fun savePopularMoviesToCache(listOfMovies: List<PopularMovie>) {
        popularMovieDao.clearTable()
        popularMovieDao.addMovies(listOfMovies)
    }

I am mostly interested in MainRepository and its implementation of fetchPopularMovies As you can see, this one function does not have only 1 job, in fact, it does multiple things depending on the situation it is in.

  1. It fetches movies using Retrofit Interface. -> Exactly what it should do Then it either: 2.1. Saves movies to cache or 2.2. Gets movies from cache

Therefore it does not really do 1 thing as it should if we follow the Separation of concerns if I understand correctly.

Here is how I think a correct implementation could look:

  1. In MainRepository, create 2 separate functions. 1 for saving movies to cache, 1 for getting movies from cache

  2. In ViewModel do the same thing and create 2 separate functions. 1 for saving to cache. 1 for getting from cache

  3. In Fragment, where we observe. If success -> call saveCache function in ViewModel if Failure -> call getCache in ViewModel

Please give me some feedback on my thought process. I would really appreciate this!

\$\endgroup\$
1
  • \$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Commented Oct 7, 2020 at 14:17

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.