Rightpoint / android-template

A `cookiecutter` template for Android projects
28 stars 4 forks source link

Remove Realm from Template #41

Closed octohub closed 6 years ago

octohub commented 6 years ago

I don't think the added effort is worth it in order to support both Room and Realm. There is a lot of ugly if branching in the template that can be removed if we only support Room. Yes, we can maintain different branches of the template to support this, but this also feels like a band aid.

I'm thinking about the future of this project and keeping it as easy to maintain as possible. Supporting only Room, the first party persistence solution, is a way to do this. And there is nothing preventing developers from simply replacing Room with Realm or any other persistence layer post generation.

@tetedoie @jonduran3000 I would love your thoughts on this.

octohub commented 6 years ago

Just had a holy war over here with @tetedoie and @arjunvekariyagithub. To get everyone up to speed, we think it is best to simply ask the user if they would like to configure Room with the project. This effectively changes the choice from Room or Realm to Room or nothing. We think this is a good middle ground because it won't force Room on the user.

I'm still of the opinion that we should just default to integrating Room and not even ask the user, because the majority of times a persistence layer will be needed. @tetedoie Wants to avoid forcing it on the user, which I hear.

What does everyone think?

jkim24 commented 6 years ago

I agree with @tetedoie here. If users don't prefer Room for whatever reason, we shouldn't force it upon them. My worry is that it could discourage users from using the template.

octohub commented 6 years ago

I hear that @jkim24 but three things I'd like to counter with:

  1. The goal of the template is to handle the majority use case. Most apps need a persistence layer and most users will be good with Room as it is the first party Google built and maintained persistence solution on Android. I want to save the majority of users time rather than focus on the minority who would like to use a solution besides Room or not one at all. We should optimize for the general, not the exception.

  2. The main goal of the template is to make Android development at Rightpoint better. I understand this is open source and I know those outside Rightpoint can get a lot of value from this. But I want to make sure we are optimizing for the Rightpoint use case first.

  3. Bundling Room in the template is not forcing users to use it. It can easily be removed. There is hardly any code associated with it in the generated project.

Bottom line, if we don't bundle Room, more users will have to spend time configuring a persistence layer than will have to remove it if we do bundle Room.

Let the holy war continue.

jkim24 commented 6 years ago

Some good points there @octohub, especially the second one. I'm game to leave it in there.

arjunvekariyagithub commented 6 years ago

@octohub @jkim24 @tetedoie I just gave it some more thoughts. I would say defaulting to only Room instead of Room or nothing would be more convenient as:

  1. Considering more then 90% of dev will use some or other persistance method, we should provide some default cache configuration which is reliable and most preferable (i.e Room because it's from Google which develops Android and thus might always be reliable & maintained, and remain compatible even with future Android versions). Now, Let's say dev wants to use other persistance library (i.e Realm) but not Room. Here, while generating project from template if dev sees option as Room or nothing he is most likely to select nothing because he don't need Room. And then, he will need to setup entire cache module from scratch. Also if there is no cache module in generated project, dev might choose not to have separate module for cache because of lack of knowledge about clean architecture so i believe we should always keep default cache module to enforce clean architecture. Due to this reasons, if we just default to Room, dev will always have cache module with Room configuration and if he wants to use other persistance library (i.e Realm) then he simply need to change configuration from Room to whatever he choose to use.

  2. For remaining 10% devs who don't want to use persistance, they can simply remove cache module in one click. So, because of only 10% devs, we should not make 90% of dev's life miserable.

let me know your thoughts?

octohub commented 6 years ago

@arjunvekariyagithub Great points brother, thank you for sharing. Thanks @jkim24 for pushing back, this dialogue is important.

@jonduran3000 what are your thoughts brother?

jonduran3000 commented 6 years ago

I personally want to attempt a branching structure that would allow us to support both but as separate branches that we can reference. However, I discussed this with key members of the iOS team who attempted this structure with their template and they ended up rejecting it because of the level of effort it took to maintain. I don't want there to be a situation where there's no default but I do want there to be a default because if caching truly isn't needed (which I doubt) then the project lead can just delete it. We just need to come to an agreement on how we achieve this. It may just be a branching structure and a particular developer has to commit to maintaining their specific branch.

octohub commented 6 years ago

Thank you @jonduran3000. Agreed that I think the branching structure is going to be difficult to maintain. I want to keep the android-template as simple as possible.

I think the solution is to make Room the default. For the edge case where Room is not used devs can simply drop in another lib.

Are we good to remove the Realm option? Please react with a thumbs up or thumbs down. If you react with a thumbs down please offer an alternative solution.

cc @jkim24 @tetedoie @arjunvekariyagithub

octohub commented 6 years ago

Handled: https://github.com/Raizlabs/android-template/pull/43