android / architecture-samples

A collection of samples to discuss and showcase different architectural tools and patterns for Android apps.
Apache License 2.0
44.5k stars 11.65k forks source link

[todo-mvvm-rxjava] getTask() has perhaps a missing filter #407

Closed mahmed1987 closed 7 years ago

mahmed1987 commented 7 years ago

Line Number 263 at https://github.com/googlesamples/android-architecture/blob/dev-todo-mvvm-rxjava/todoapp/app/src/main/java/com/example/android/architecture/blueprints/todoapp/data/source/TasksRepository.java

has the following return

`return Observable.concat(localTask, remoteTask).first() .map(task -> { if (task == null) { throw new NoSuchElementException("No task found with taskId " + taskId); } return task; });

`

aren't we missing a filter call before first ? . I can't make sense of this without the filter applied. Please elaborate if the filter is correctly not applied here.

mahmed1987 commented 7 years ago
return Observable.concat(localTask, remoteTask).**filter(task -> task != null)**.first()
                .map(task -> {
                    if (task == null) {
                        throw new NoSuchElementException("No task found with taskId " + taskId);
                    }
                    return task;
                });

I believe it should be like above ? . I am actually new to Rx so I am not sure if its something you guys missed or it is something intentionally left.

florina-muntenescu commented 7 years ago

Code changed meanwhile. Not applicable anymore.