Some feedback

[ajarl]

Here's some feedback from our interview.

• NetworkState: Could be a sealed class hierarchy depending on how it's used, e.g. data class Idle(data), object Loading, data class Error(error).
• Model classes: Could use more descriptive names. Also it might be advantageous to avoid using these types all the way into the UI.
• WeatherService: Consider using suspend fun instead of returning Deferred
• LocationUtil: The MutableLiveDatas should perhaps be kept private and non-mutable LiveDatas exposed. Last location and current location can probably be combined.
  • The getAddressByCoordinates function should likely be a suspend function that returns the address. This allows callers to properly cancel when needed.
  • getFromLocation shouldn't happen on the main thread; use withContext for the blocking call to switch dispatcher
• CurrentLocationFragment: consider moving loadData into the ViewModel instead
• DailyRecyclerViewAdapter: mValues can be a non-mutable list instead
• CurrentLocationViewModel: The Job is never cancelled. The scope uses Dispatchers.IO, it would make more sense to default to Main. Ideally you should use viewModelScope.
  • The ViewModel should be responsible for listening for location updates and automatically pushing new forecasts to the LiveData. Right now the view has to inform the ViewModel about the location updates.
  • The getForecast function's units parameter should be the enum type and not a string to avoid passing invalid values

[srrollin]

@ajarl Thank you for these awesome suggestions. I'll apply them.