From 40944441f899c763f218f255354189e1aece5202 Mon Sep 17 00:00:00 2001 From: Saul M Date: Sat, 19 Dec 2015 17:46:56 +0100 Subject: [PATCH 1/4] Change the Usecase interface to an abstract class --- .../main/java/saulmm/avengers/CharacterDetailsUsecase.java | 2 +- .../src/main/java/saulmm/avengers/GetCharactersUsecase.java | 2 +- .../src/main/java/saulmm/avengers/GetCollectionUsecase.java | 2 +- domain/src/main/java/saulmm/avengers/Usecase.java | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java b/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java index cd154f8..5160729 100644 --- a/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java +++ b/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java @@ -13,7 +13,7 @@ import saulmm.avengers.entities.MarvelCharacter; import saulmm.avengers.repository.CharacterRepository; -public class CharacterDetailsUsecase implements Usecase { +public class CharacterDetailsUsecase extends Usecase { private final CharacterRepository mRepository; private final Scheduler mResultsThread; private final Scheduler mExecutorThread; diff --git a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java index 511732a..fb14532 100644 --- a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java +++ b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java @@ -9,7 +9,7 @@ import saulmm.avengers.entities.MarvelCharacter; import saulmm.avengers.repository.CharacterRepository; -public class GetCharactersUsecase implements Usecase> { +public class GetCharactersUsecase extends Usecase> { public final static int DEFAULT_CHARACTERS_LIMIT = 20; private int mCharactersLimit = DEFAULT_CHARACTERS_LIMIT; diff --git a/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java b/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java index 9ec91e9..9b71814 100644 --- a/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java +++ b/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java @@ -6,7 +6,7 @@ import saulmm.avengers.entities.CollectionItem; import saulmm.avengers.repository.CharacterRepository; -public class GetCollectionUsecase implements Usecase> { +public class GetCollectionUsecase extends Usecase> { private final CharacterRepository mRepository; private final int mCharacterId; diff --git a/domain/src/main/java/saulmm/avengers/Usecase.java b/domain/src/main/java/saulmm/avengers/Usecase.java index 13fa8f4..b8f6071 100644 --- a/domain/src/main/java/saulmm/avengers/Usecase.java +++ b/domain/src/main/java/saulmm/avengers/Usecase.java @@ -7,7 +7,7 @@ import rx.Observable; -public interface Usecase { +public abstract class Usecase { - Observable execute(); + abstract Observable execute(); } From 6dd2b7d7ab985475e86d4b6df77320eb1dcd3917 Mon Sep 17 00:00:00 2001 From: Saul M Date: Sat, 19 Dec 2015 18:58:07 +0100 Subject: [PATCH 2/4] Changed usecases structure --- .../modules/AvengerInformationModule.java | 5 +- .../presenters/CharacterListPresenter.java | 2 +- .../mvp/presenters/CollectionPresenter.java | 5 +- .../avengers/CharacterDetailsUsecase.java | 14 ++---- .../saulmm/avengers/GetCharactersUsecase.java | 49 +++++++------------ .../saulmm/avengers/GetCollectionUsecase.java | 18 ++++--- .../main/java/saulmm/avengers/Usecase.java | 10 +++- 7 files changed, 46 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/saulmm/avengers/injector/modules/AvengerInformationModule.java b/app/src/main/java/saulmm/avengers/injector/modules/AvengerInformationModule.java index 859d5be..34b5467 100644 --- a/app/src/main/java/saulmm/avengers/injector/modules/AvengerInformationModule.java +++ b/app/src/main/java/saulmm/avengers/injector/modules/AvengerInformationModule.java @@ -23,9 +23,8 @@ public AvengerInformationModule(int characterId) { mCharacterId = characterId; } - @Provides @Activity CharacterDetailsUsecase provideGetCharacterInformationUsecase (CharacterRepository repository, @Named("ui_thread") Scheduler uiThread, - @Named("executor_thread") Scheduler executorThread) { - return new CharacterDetailsUsecase(mCharacterId, repository, uiThread, executorThread); + @Provides @Activity CharacterDetailsUsecase provideGetCharacterInformationUsecase (CharacterRepository repository) { + return new CharacterDetailsUsecase(mCharacterId, repository); } @Provides @Activity GetCollectionUsecase provideGetCharacterComicsUsecase (CharacterRepository repository) { diff --git a/app/src/main/java/saulmm/avengers/mvp/presenters/CharacterListPresenter.java b/app/src/main/java/saulmm/avengers/mvp/presenters/CharacterListPresenter.java index 21ecf92..6eee80c 100644 --- a/app/src/main/java/saulmm/avengers/mvp/presenters/CharacterListPresenter.java +++ b/app/src/main/java/saulmm/avengers/mvp/presenters/CharacterListPresenter.java @@ -80,7 +80,7 @@ public void askForNewCharacters() { mAvengersView.showLoadingMoreCharactersIndicator(); mIsTheCharacterRequestRunning = true; - mCharactersSubscription = mCharactersUsecase.executeIncreasingOffset() + mCharactersSubscription = mCharactersUsecase.execute() .subscribe(this::onNewCharactersReceived, this::onNewCharactersError); } diff --git a/app/src/main/java/saulmm/avengers/mvp/presenters/CollectionPresenter.java b/app/src/main/java/saulmm/avengers/mvp/presenters/CollectionPresenter.java index 362340e..374b306 100644 --- a/app/src/main/java/saulmm/avengers/mvp/presenters/CollectionPresenter.java +++ b/app/src/main/java/saulmm/avengers/mvp/presenters/CollectionPresenter.java @@ -37,9 +37,8 @@ public void attachView(View v) { @Override public void onCreate() { - mGetCollectionUsecase.execute(mCollectionType) - .subscribeOn(Schedulers.newThread()) - .observeOn(AndroidSchedulers.mainThread()) + mGetCollectionUsecase.setType(mCollectionType); + mGetCollectionUsecase.execute() .subscribe(this::onCollectionItemsReceived); } diff --git a/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java b/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java index 5160729..f4e9b27 100644 --- a/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java +++ b/domain/src/main/java/saulmm/avengers/CharacterDetailsUsecase.java @@ -15,25 +15,17 @@ public class CharacterDetailsUsecase extends Usecase { private final CharacterRepository mRepository; - private final Scheduler mResultsThread; - private final Scheduler mExecutorThread; private int mCharacterId; @Inject public CharacterDetailsUsecase(int characterId, - CharacterRepository repository, - @Named("ui_thread") Scheduler uiThread, - @Named("executor_thread") Scheduler executorThread) { + CharacterRepository repository) { mCharacterId = characterId; mRepository = repository; - mResultsThread = uiThread; - mExecutorThread = executorThread; } @Override - public Observable execute() { - return mRepository.getCharacter(mCharacterId) - .observeOn(mResultsThread) - .subscribeOn(mExecutorThread); + public Observable buildObservable() { + return mRepository.getCharacter(mCharacterId); } } diff --git a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java index fb14532..d50ff28 100644 --- a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java +++ b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java @@ -11,50 +11,39 @@ public class GetCharactersUsecase extends Usecase> { public final static int DEFAULT_CHARACTERS_LIMIT = 20; - private int mCharactersLimit = DEFAULT_CHARACTERS_LIMIT; - private final Scheduler mResultsThread; - private final Scheduler mExecutorThread; - private final CharacterRepository mRepository; private int mCurrentOffset; - @Inject public GetCharactersUsecase(CharacterRepository repository, - @Named("ui_thread") Scheduler uiThread, - @Named("executor_thread") Scheduler executorThread) { + @Inject @Named("ui_thread") Scheduler uiThread; + @Inject @Named("executor_thread") Scheduler executorThread; + + @Inject public GetCharactersUsecase(CharacterRepository repository) { mRepository = repository; - mResultsThread = uiThread; - mExecutorThread = executorThread; } @Override - public Observable> execute() { - return mRepository.getCharacters(mCurrentOffset) - .observeOn(mResultsThread) - .subscribeOn(mExecutorThread); - } - - public Observable> executeIncreasingOffset() { - mCurrentOffset += mCharactersLimit; - + public Observable> buildObservable() { return mRepository.getCharacters(mCurrentOffset) - .observeOn(mResultsThread) - .subscribeOn(mExecutorThread) - .doOnError(new Action1() { - @Override - public void call(Throwable throwable) { - mCurrentOffset -= mCharactersLimit; - } - }); + .observeOn(uiThread) + .subscribeOn(executorThread) + .doOnError(new Action1() { + @Override + public void call(Throwable throwable) { + mCurrentOffset -= mCharactersLimit; + } + }); } - public void setCharactersLimit(int charactersLimit) { - mCharactersLimit = charactersLimit; + @Override + public Observable> execute() { + increaseOffset(); + return super.execute(); } - public void setCurrentOffset(int currentOffset) { - mCurrentOffset = currentOffset; + public void increaseOffset() { + mCurrentOffset += mCharactersLimit; } public int getCurrentOffset() { diff --git a/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java b/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java index 9b71814..a1df2d4 100644 --- a/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java +++ b/domain/src/main/java/saulmm/avengers/GetCollectionUsecase.java @@ -9,22 +9,24 @@ public class GetCollectionUsecase extends Usecase> { private final CharacterRepository mRepository; private final int mCharacterId; + private String mType; @Inject public GetCollectionUsecase(int characterId, CharacterRepository repository) { mRepository = repository; mCharacterId = characterId; } - @Override - public Observable> execute() { - return null; - } - - public Observable> execute(String type) { + public void setType(String type) { if (!type.equals(CollectionItem.COMICS) && !type.equals(CollectionItem.EVENTS) && !type.equals( - CollectionItem.SERIES) && !type.equals(CollectionItem.STORIES)) + CollectionItem.SERIES) && !type.equals(CollectionItem.STORIES)) + throw new IllegalArgumentException("Collection type must be events|series|comics|stories"); - return mRepository.getCharacterCollection(mCharacterId, type); + mType = type; + } + + @Override + public Observable> buildObservable() { + return mRepository.getCharacterCollection(mCharacterId, mType); } } diff --git a/domain/src/main/java/saulmm/avengers/Usecase.java b/domain/src/main/java/saulmm/avengers/Usecase.java index b8f6071..c5c64f2 100644 --- a/domain/src/main/java/saulmm/avengers/Usecase.java +++ b/domain/src/main/java/saulmm/avengers/Usecase.java @@ -5,9 +5,17 @@ */ package saulmm.avengers; +import javax.inject.Inject; +import javax.inject.Named; + import rx.Observable; +import rx.Scheduler; +import rx.Subscriber; public abstract class Usecase { + public abstract Observable buildObservable(); - abstract Observable execute(); + public Observable execute() { + return buildObservable(); + } } From 0ab9a4f64218b18e6b501e7e67835a4775c7f2e4 Mon Sep 17 00:00:00 2001 From: Saul M Date: Mon, 21 Dec 2015 01:10:42 +0100 Subject: [PATCH 3/4] Now the schedulers are passed through constructor --- .../saulmm/avengers/GetCharactersUsecase.java | 7 +++--- .../test/java/GetCharacterDetailsTest.java | 2 +- .../test/java/GetCharactersUsecaseTest.java | 23 +++++++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java index d50ff28..a1b3493 100644 --- a/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java +++ b/domain/src/main/java/saulmm/avengers/GetCharactersUsecase.java @@ -15,12 +15,13 @@ public class GetCharactersUsecase extends Usecase> { private final CharacterRepository mRepository; private int mCurrentOffset; - @Inject @Named("ui_thread") Scheduler uiThread; - @Inject @Named("executor_thread") Scheduler executorThread; + Scheduler uiThread, executorThread; - @Inject public GetCharactersUsecase(CharacterRepository repository) { + @Inject public GetCharactersUsecase(CharacterRepository repository, @Named("ui_thread") Scheduler uiThread, @Named("executor_thread") Scheduler executorThread) { mRepository = repository; + this.uiThread = uiThread; + this.executorThread = executorThread; } @Override diff --git a/domain/src/test/java/GetCharacterDetailsTest.java b/domain/src/test/java/GetCharacterDetailsTest.java index 63cc2fe..451ea7e 100644 --- a/domain/src/test/java/GetCharacterDetailsTest.java +++ b/domain/src/test/java/GetCharacterDetailsTest.java @@ -41,7 +41,7 @@ public class GetCharacterDetailsTest { } private CharacterDetailsUsecase givenACharacterUsecase() { - return new CharacterDetailsUsecase(FAKE_CHARACTER_ID, mRepository, mockScheduler, mockScheduler); + return new CharacterDetailsUsecase(FAKE_CHARACTER_ID, mRepository); } private Observable getFakeCharacterObservable() { diff --git a/domain/src/test/java/GetCharactersUsecaseTest.java b/domain/src/test/java/GetCharactersUsecaseTest.java index 0895e7e..db7a8ae 100644 --- a/domain/src/test/java/GetCharactersUsecaseTest.java +++ b/domain/src/test/java/GetCharactersUsecaseTest.java @@ -17,12 +17,15 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.when; import static saulmm.avengers.GetCharactersUsecase.DEFAULT_CHARACTERS_LIMIT; public class GetCharactersUsecaseTest { @Mock CharacterRepository mockRepository; + @Mock Scheduler mockUiScheduler; + @Mock Scheduler mockExecutorScheduler; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -36,26 +39,26 @@ public class GetCharactersUsecaseTest { @Test public void testThatCharactersUsecaseIncrementsOffset() throws Exception { GetCharactersUsecase charactersUsecase = givenACharactersUsecase(); - when(mockRepository.getCharacters(anyInt())).thenReturn(getFakeObservableCharacterList()); - charactersUsecase.executeIncreasingOffset(); - assertThat(charactersUsecase.getCurrentOffset(), is(DEFAULT_CHARACTERS_LIMIT)); + charactersUsecase.execute(); + charactersUsecase.execute(); + charactersUsecase.execute(); + + assertThat(charactersUsecase.getCurrentOffset(), is(DEFAULT_CHARACTERS_LIMIT * 3)); } @Test public void testThatCharactersUsecaseWithOffsetIsCalledOnce() throws Exception { GetCharactersUsecase charactersUsecase = givenACharactersUsecase(); - int fakeCurrentOffset = 20; + when(mockRepository.getCharacters(anyInt())).thenReturn(getFakeObservableCharacterList()); + + charactersUsecase.execute(); - when(mockRepository.getCharacters(fakeCurrentOffset)).thenReturn(getFakeObservableCharacterList()); - charactersUsecase.executeIncreasingOffset(); - - Mockito.verify(mockRepository, only()).getCharacters(fakeCurrentOffset); + Mockito.verify(mockRepository, only()).getCharacters(anyInt()); } private GetCharactersUsecase givenACharactersUsecase() { - return new GetCharactersUsecase(mockRepository, - Schedulers.io(), Schedulers.newThread()); + return new GetCharactersUsecase(mockRepository, mockUiScheduler, mockExecutorScheduler); } private Observable> getFakeObservableCharacterList() { From 22d40cc629accd8a3430e7777c930ea09c59e09d Mon Sep 17 00:00:00 2001 From: Saul M Date: Mon, 21 Dec 2015 01:16:00 +0100 Subject: [PATCH 4/4] Fixed broken test --- app/src/test/java/ListPresenterTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/ListPresenterTest.java b/app/src/test/java/ListPresenterTest.java index acf88ed..b116931 100644 --- a/app/src/test/java/ListPresenterTest.java +++ b/app/src/test/java/ListPresenterTest.java @@ -57,7 +57,7 @@ public class ListPresenterTest { @Test public void testThatPresenterShowsALightErrorLoadingMoreCharacters() throws Exception { CharacterListPresenter listPresenter = givenAListPresenter(); - when(mockGetCharacterUsecase.executeIncreasingOffset()).thenReturn(Observable.error(new Exception())); + when(mockGetCharacterUsecase.execute()).thenReturn(Observable.error(new Exception())); listPresenter.askForNewCharacters(); verify(mockCharacterListView, times(1)).showLightError(); @@ -66,10 +66,10 @@ public class ListPresenterTest { @Test public void testThatPresenterRequestMoreCharacters() throws Exception { CharacterListPresenter listPresenter = givenAListPresenter(); - when(mockGetCharacterUsecase.executeIncreasingOffset()).thenReturn(getFakeObservableCharacterList()); + when(mockGetCharacterUsecase.execute()).thenReturn(getFakeObservableCharacterList()); listPresenter.askForNewCharacters(); - verify(mockGetCharacterUsecase, only()).executeIncreasingOffset(); + verify(mockGetCharacterUsecase, only()).execute(); } private ArrayList givenAFakeCharacterList() {