Proposal of Introducing DI

[mandaiy]

This is a proposal issue for introducing DI container (Dependency Injection) For diverse opinions, please feel free to leave any comment, @amiq11.

TL;DR

• I hate the current structure of ...ActivityBase and Flavor...Activity
• I want to restructure them by splitting ...ActivityBase into common logic and flavor-specific logic
• I want to introduce Dagger2

  Background

Currently we are going to implement product flavors. There are two main features of product flavors: determining java files and merging xml.

(You can skip below 2 sections if you know product flavors)

determining java files

With product flavors, you can change files (or classes) by placing corresponding directories; suppose you define 2 flavors (flavor1, flavor2) and have flavor1/java/com/example/Test.java, and flavor2/java/com/example/Test.java, and the code in main/java/com/example/Main.java refers to Test class, then the Test class is referred with accordance of the product flavor in build-time.

merging xml

You can define AndroidManifest.xml files for each product flavor and one main. main does not belong to flavors, but it defines common resources (xml, java, images). If you have AndroidManifest.xml in main and flavor1, Android Studio merges the two xml files in build-time. Why this is useful is that you can change resources according to the flavor. A launch activity is defined in the manifest xml file. So you can choose the launch activity by defining the manifest xml in the flavor.

Why we use it and How we used it

Why this feature is needed for us is because:

• we have to support multiple types of robots, each of which needs slightly different code (e.g., types of block, types of communication methods)
• but code-duplication has to be minimized

That's why we are going to use productFlavors. We have nxt, ev3, and pile robots so we'll have 3 product flavors.

In our current code, we we define TitleActivity for each flavor and specify launch activities in manifest xml files for each flavor. With the xml files, the launch activity of each app can start properly. And for programming (block-manipulation) and execution (communication with robots) activities, we define ProgrammingActivityBase and ExecutionActivityBase respectively, and then in each flavor these classes are inherited according to the flavor (e.g., for nxt, NxtProgrammingActivity and NxtExecutioinActivity).

The code of ProgrammingActivityBase is like below:

public abstract class ProgrammingActivityBase extends Activity {

    /* these methods are implemented in concrete classes */
    protected abstract Intent getIntentForBlockList();

    protected abstract Intent getIntentForExecution();

    /* the rest is common for all flavors */
    private void goToExecution() {
        Intent intent = getIntetnForExecution();
        ...
    }
    ...
};

Then in NxtProgrammingActivity is like below:

public class NxtProgrammingActivity extends ProgrammingActivityBase {

    @Override
    protected Intent getIntentForBlockList() {
        return new NxtBlockIntent();
    }

    @Override
    protected Intent getIntentForExecution() {
        return new NxtExecutionIntent();
    }
}

Problem Statement

What I am concering is that the structure seems awkward and the base classes are too huge for abstract class. In my opinion, the straightforward way for this situation is that defining interface for flavor-specific logic (intent-provider for programming) and injecting flavor-specific-implementation into the interface.

So what I want to suggest is like below:

public class ProgrammingActivity extends Activity {
    private IntentProvider intentProvider;

    private void goToExecution() {
        Intent intent = intentProvider.forExecution();
        ...
    }
}

public interface IntentProvider {
    Intent forExecution();
}

public class NxtIntentProvider {
    @Override
    public Intent forExecution() {
        return ...
    }
}

By splitting code like above, the code gets

• better organized
• easier to be mocked
• easier for new-comers to understand what is needed to be implemented for other flavors.

And this proposal also solves #31.

Coping with dependency by DI container

If you feel my proposal sounds good, you might wonder how the interfaces are injected. That kind of problems are called Dependency Injection. There are a number of pages describing this problem, so please google it.

For Android projects, there is a cool DI container called Dagger2. So I want to use this for our android project.

I tested the feasibility of my proposal in here

By using Dagger2, the code would be like below:

public class ProgrammingActivity extends Activity {

    @Inject
    IntentProvider intentProvider;

    /* same as above */
}

How cool, isn't it? Thank you for reading this too long issue.

[tiwanari]

I'm sorry for late response. I needed time to become familiar with DI and Dagger2. I'm really happy to know them and agree with the proposal 👍

So, in my understand, we can remove Base suffix like from ProgrammingActivityBase to ProgrammingActivity because we can change the contents of provider methods - thus, we don't have to change anything in ProgrammingActivity - with product flavors and Dagger2 helps us make our codes simple or organized, right?

BTW, this is just a question, can we use the IntentProvider just with product flavor - without Dagger2 - by sharing the same name in flavor1, flavor2 like flavor1/com/example/IntentProvider and flavor1/com/example/IntentProvider?

[mandaiy]

Hey all.

So, in my understand, we can remove Base suffix like from ProgrammingActivityBase to ProgrammingActivity because we can change the contents of provider methods - thus, we don't have to change anything in ProgrammingActivity - with product flavors and Dagger2 helps us make our codes simple or organized

Yeah, right.

can we use the IntentProvider just with product flavor - without Dagger2 - by sharing the same name in flavor1, flavor2 like flavor1/com/example/IntentProvider and flavor1/com/example/IntentProvider?

That's true. BUT we should NOT directly depend on concrete classes; but rely on interfaces The interface layers make code durable, easier to be tested and mocked. That's the motivation of introducing DI containers. So we can enjoy that features by using DI containers.

And for Android platforms, there are a few DI containers other than Dagger2:

• Roboguice - deprecated
• ButterKnife - only for views
• proton - not active

So I suggest using Dagger2.

[tiwanari]

I see and totally agree with you :yum:

[makotoshimazu]

Sorry for late reviewing! Could you wait a moment because I'm still not familiar with the DI and the Dagger2?

[tiwanari]

Hi @amiq11,

FYI, as we discussed above, DI does more things that we need to solve the ugly ~Base class problem related to product flavor, and this is one of the core problems of our product. DI and Dagger2 are a solution of it. They can specify the frame of our codes, i.e how to write codes related to product flavor.

[makotoshimazu]

DI pattern itself looks good though current Base class architecture is straightforward and not so bad (I think it's not ugly). However, I'm still not sure about how much benefits we can obtain by using Dagger2 for DI container. Dagger2 has their own annotations like @Inject, and I'm worried that those uncommon syntax might introduce some kind of complexity. I think we should confirm if we'll be able to obtain enough benefits to use the library. Is there any benefits other than simpleness to write? (ex: integration of testing framework)

[tiwanari]

I want to discuss this issue as the following 2 or 3 separated topics.

• *Base classes in main flavor
  • In my opinion, we should remove *Base inheritance.
• (DI pattern itself)
  • This is a solution of the above problem
  • DI itself got our agreements, right?
• Using Dagger2 as a DI container
  • As @amiq11 said, this may introduce a kind of complexity with its original annotations.

Let me describe my feeling for each topic in more detail below.

*Base classes

As @amiq11 said, the architecture is a straight forward way. However, in my opinion, we should replace it.

We use the architecture originally for adjusting *Base classes to each app but, at present, it is not good software design to do that because I think Inheritance is too much for this case. In the design, inherited classes are forced to implement methods just related to their flavor, and (almost?) all methods implemented in their superclass are not necessary to implement them - so they are noisy when a developer implements only flavor-related methods (we know we will be interested with superclasses when we use inheritance while we can concentrate on implementation of core methods specified with interfaces :P).

From a different perspective, I think this is a kind of composition vs inheritance problem (yeah, I know it is a bit different from the original problem but they have almost the same pros & cons). In the context, I also prefer to use composition (i.e., remove the *Base inheritance) for many reasons (please see: Composition vs Inheritance).

For these reasons, I agree with @myusak's proposal.

DI pattern

This is the next step of composition vs inheritance problem and it proposes a way how to make components (based on a conclusion "composition is better" in this context). We have already shared positive feelings for this pattern, right? At least, I think this is better than making components in a constructor or so because of reduction of coupling between classes and the most profitable point of DI is that it makes testing easier, I think.

Dagger2

This is controvertible yet. The main problem we are discussing here is that its original annotations may introduce a kind of complexity - Java has many annotations and actually they sometimes confuse me :(.

btw, if you agree to introduce DI in our products for some reasons shown above such as coupling reduction / easier testing, the next topic is which framework we should use. As we discussed above, we can do DI just with product flavors but interface-based ways also make testing easier. I agree with this idea.

There are many alternatives indeed, however, maybe, a reason why we want to use Dagger2 is because it is the best DI container at this time.

Please share your feelings :)

[makotoshimazu]

I feel DI pattern is not so special and agree to introduce DI pattern. However, I'm still not sure if we should use Dagger2. IMO, we can implement such pattern without frameworks, and it would be not so difficult and not so complicated. If the benefit from Dagger2 is only less number of lines, I think it's better to implement interfaces manually. What do you think about that?

[mandaiy]

I should point out @amiq11's misunderstanding.

Dagger2 has their own annotations like @Inject

It's not Dagger2-specific annotation. @Inject, @Singleton, and other annotation Dagger2 uses are defined in JSR-330. i.e., @Inject is in package javax.inject. In other words, these annotations are compatible with other DI containers to some extent.

In my opinion, by using DI container properly, we are able to separate classes properly. This means these classes get easier to be tested and mocked. And this is the main benefit of introducing DI container.

[mandaiy]

I guess @amiq11 is wondering that our app gets less-generic. But for Java, it seems natural to introduce such frameworks especially for bigger projects. Then they defined common annotation for compatibility. And our app gets bigger and it's time to introduce DI framework.

I feel benefit from Dagger2 is only less number of lines is not minor but major benefit. I do not want write code if possible, seriously.

[makotoshimazu]

Got it. I didn't know that the annotations are standardized.

I guess @amiq11 is wondering that our app gets less-generic.

Right, that's my main concern. If it's not a problem, I think we can go ahead.

[tiwanari]

I learned many things here. Thank you for sharing ideas and knowledge :sunglasses:

Here, this is conclusion (Please confirm it):

• Remove *Base
  • Use DI to do that (it can be applied to other classes)
  • Use Dagger2 as a DI container

If there is no strong objection, let's make a step forward :) What do you think about how we should do your proposal, @myusak ? Will you make a PR? or make changes step by step together?

[mandaiy]

I'll do this because it's my proposal. I believe this is not big deal. From time to time, I'll ask you about design of interfaces.

[tiwanari]

Thanks! Please feel free to talk with me and it's also okay to pass me a task related to this if you are too busy or so :)