Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature to Feature binding #103

Open
linyaDev opened this issue Jul 25, 2019 · 7 comments
Open

Feature to Feature binding #103

linyaDev opened this issue Jul 25, 2019 · 7 comments
Labels
question Further information is requested

Comments

@linyaDev
Copy link

linyaDev commented Jul 25, 2019

Hi.
In best practices you wrote next code

class BootstrapperImpl(
        private val feature1: Feature1
    ) : Bootstrapper<Wish> {
        override fun invoke(): Observable<Wish> =
            feature1.news.map { SomeWishOfFeature2  }
    }

But i can't understand best way to do this map , because news is ObservableSource and don't have map function.
Maybe you have some ktx extensions for this?

Now i do this.

class  BootstrapperImpl(
       private val feature: Feature1
   ) : Bootstrapper<Wish> {
       override fun invoke(): Observable<Wish> =
          Observable.create {
              emitter ->
              val newsListener = object : Observer<Feature1.News>{
                  override fun onNext(news: Feature1.News) {
                      when(news){
                          else->emitter.onNext(Wish.Update())
                      }
                  }
                  override fun onComplete() {
                     emitter.onComplete() 
                  }
                  override fun onSubscribe(d: Disposable) {}
                  override fun onError(e: Throwable) {}
              }
              feature.news.subscribe(newsListener)
           }
   }
@amihusb
Copy link

amihusb commented Jul 25, 2019

Hi @LinKR.
I guess this is a mistake, to solve this case you should simply do like this:

class BootstrapperImpl(
    private val feature1: Feature1
) : Bootstrapper<Wish> {

    override fun invoke(): Observable<Wish> =
        Observable.wrap(feature1.news).map { SomeWishOfFeature2  }
}

@zsoltk
Copy link
Contributor

zsoltk commented Jul 25, 2019

@LinKR The docs contain an error, thanks for pointing out. It should be as @amihusb wrote, use Observable.wrap() around any ObservableSource first.

@zsoltk
Copy link
Contributor

zsoltk commented Jul 25, 2019

Also I would rather use Binder.bind(feature1.news to feature2 using Feature1NewsToFeature2Wish) scoped bindings. Please check respective sections in docs about Binder.

@zsoltk zsoltk added the question Further information is requested label Jul 25, 2019
@VGJohn
Copy link

VGJohn commented Aug 18, 2019

@zsoltk I have an issue related to feature-to-feature binding that I'm having difficulty finding a good solution, maybe you'd have a different suggestion to get around this issue though.

Hopefully this example will explain my problem enough... There are two features; a GalleryFeature and CameraFeature each of which have their own Fragments. The GalleryFragment receives an observable list of media items to display and the CameraFragment allows the user to add more media items to their gallery. When the user takes a photo/video in the CameraFragment, the CameraFeature sends the file path for the new media item to the GalleryFeature so the new media item will appear in the GalleryFragment after the CameraFragment pops off the backstack. The GalleryFeature does a little bit of extra processing on the new media item as well so I'd like to start that without having to wait for the GalleryFragment to resume. My issue is binding these features since there is a bit of a forward referencing problem.

What I tried doing was using DI (I'm using Koin) to inject the CameraFeature into the GalleryFeature Bootstrapper as suggested above and then the same CameraFeature instance would be injected into the CameraFragment. This all works fine but...

The issue with this approach is that the CameraFeature instance will always be the same, so if I have two GalleryFragments on the backstack each with their own GalleryFeature instance, those GalleryFeatures will have the same CameraFeature instance injected by Koin. Now when the CameraFeature sends the new media item through the NewsPublisher, it will go both GalleryFeatures when I only wanted the feature of the GalleryFragment that's at the top of the stack to receive the new media item. How do I avoid this situation?

I'm not sure what's the best approach for passing data back to the "calling" feature, which in this example is the GalleryFeature. Without running into the issue of publishing a news update to all the GalleryFeatures, if there are multiple GalleryFragment instances on backstack.

As a workaround for now, I ended up forking the repo and adding the concept of a FeatureSet as way to group related Features that work together during some portion of the flow in the app. Similar concept to the NavGraph Scoped ViewModels that were recently introduced in the Jetpack Navigation Component. Each FeatureSet would have its own instances of the Features thus avoiding the scenario above, it would be similar to a ViewModelStore. My solution is quite rough at the moment so maybe I'll refine it further if there isn't a better way to get around my issue.

Any suggestions would be appreciated, thanks!

@ShikaSD
Copy link
Contributor

ShikaSD commented Aug 19, 2019

Hey @VGJohn
Looking at your issue: did you try to use fragment lifecycle as a means to solve the problem?

We have Binder specifically for this case: it connects two endpoints using some specific lifecycle, connecting or disconnecting when the lifecycle event is propagated. @zsoltk mentioned that it is better to use scoped bindings just because of that.

NOTE: lifecycle event does not necessary mean Android lifecycle, it can be anything custom (see ManualLifecycle).

For your case, I would try to connect features from the outside rather than in the bootstrapper (in the GalleryFragment or what have you).
Therefore, you will have something like:

class GalleryFragment {
    val cameraFeature by inject()
    val galleryFeature = GalleryFeature()
    
    override fun onCreate(state: Bundle?) {
        lifecycle.resumePause {
            cameraFeature.news to galleryFeature using CameraToGallery
        }
    }
}

Check out binder section for more details :)

We have tried to implement something similar to described FeatureSet before, but scrapped the idea, as it enforces tighter coupling and brings a lot of problems handling independency of features and reusability.

@VGJohn
Copy link

VGJohn commented Aug 19, 2019

@ShikaSD I tried using the Binder with the fragments lifecycle but it blocks the GalleryFeature from starting processing the new media item until the fragment lifecycle is resumed. I'll play around with making a custom lifecycle though, sounds like that would be a better solution. Thanks for the help!

@zsoltk
Copy link
Contributor

zsoltk commented Aug 19, 2019

@VGJohn I definitely suggest Binder bindings, as it helps both with automatic scoping and keeping Feature implementations decoupled. So far we've had hundreds of cases where this approach works just fine.

I didn't understand why you need lifecycle to be resumed? You can use extension methods for wider scopes. I suggest looking at com/badoo/mvicore/android/lifecycle/LifecycleExtensions.kt. Wouldn't create-destroy solve your case?

On the other hand, I should definitely update the docs to not suggest injecting Features inside one another anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants