{"action":"opened","issue":{"url":"https://api.github.com/repos/PileProject/drive/issues/45","repository_url":"https://api.github.com/repos/PileProject/drive","labels_url":"https://api.github.com/repos/PileProject/drive/issues/45/labels{/name}","comments_url":"https://api.github.com/repos/PileProject/drive/issues/45/comments","events_url":"https://api.github.com/repos/PileProject/drive/issues/45/events","html_url":"https://github.com/PileProject/drive/issues/45","id":181807495,"number":45,"title":"Proposal of Introducing DI","user":{"login":"myusak","id":5147779,"avatar_url":"https://avatars.githubusercontent.com/u/5147779?v=3","gravatar_id":"","url":"https://api.github.com/users/myusak","html_url":"https://github.com/myusak","followers_url":"https://api.github.com/users/myusak/followers","following_url":"https://api.github.com/users/myusak/following{/other_user}","gists_url":"https://api.github.com/users/myusak/gists{/gist_id}","starred_url":"https://api.github.com/users/myusak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/myusak/subscriptions","organizations_url":"https://api.github.com/users/myusak/orgs","repos_url":"https://api.github.com/users/myusak/repos","events_url":"https://api.github.com/users/myusak/events{/privacy}","received_events_url":"https://api.github.com/users/myusak/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/PileProject/drive/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"open","locked":false,"assignee":{"login":"tiwanari","id":2739915,"avatar_url":"https://avatars.githubusercontent.com/u/2739915?v=3","gravatar_id":"","url":"https://api.github.com/users/tiwanari","html_url":"https://github.com/tiwanari","followers_url":"https://api.github.com/users/tiwanari/followers","following_url":"https://api.github.com/users/tiwanari/following{/other_user}","gists_url":"https://api.github.com/users/tiwanari/gists{/gist_id}","starred_url":"https://api.github.com/users/tiwanari/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiwanari/subscriptions","organizations_url":"https://api.github.com/users/tiwanari/orgs","repos_url":"https://api.github.com/users/tiwanari/repos","events_url":"https://api.github.com/users/tiwanari/events{/privacy}","received_events_url":"https://api.github.com/users/tiwanari/received_events","type":"User","site_admin":false},"assignees":[{"login":"tiwanari","id":2739915,"avatar_url":"https://avatars.githubusercontent.com/u/2739915?v=3","gravatar_id":"","url":"https://api.github.com/users/tiwanari","html_url":"https://github.com/tiwanari","followers_url":"https://api.github.com/users/tiwanari/followers","following_url":"https://api.github.com/users/tiwanari/following{/other_user}","gists_url":"https://api.github.com/users/tiwanari/gists{/gist_id}","starred_url":"https://api.github.com/users/tiwanari/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiwanari/subscriptions","organizations_url":"https://api.github.com/users/tiwanari/orgs","repos_url":"https://api.github.com/users/tiwanari/repos","events_url":"https://api.github.com/users/tiwanari/events{/privacy}","received_events_url":"https://api.github.com/users/tiwanari/received_events","type":"User","site_admin":false},{"login":"amiq11","id":3426676,"avatar_url":"https://avatars.githubusercontent.com/u/3426676?v=3","gravatar_id":"","url":"https://api.github.com/users/amiq11","html_url":"https://github.com/amiq11","followers_url":"https://api.github.com/users/amiq11/followers","following_url":"https://api.github.com/users/amiq11/following{/other_user}","gists_url":"https://api.github.com/users/amiq11/gists{/gist_id}","starred_url":"https://api.github.com/users/amiq11/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/amiq11/subscriptions","organizations_url":"https://api.github.com/users/amiq11/orgs","repos_url":"https://api.github.com/users/amiq11/repos","events_url":"https://api.github.com/users/amiq11/events{/privacy}","received_events_url":"https://api.github.com/users/amiq11/received_events","type":"User","site_admin":false}],"milestone":null,"comments":0,"created_at":"2016-10-08T06:08:28Z","updated_at":"2016-10-08T06:08:28Z","closed_at":null,"body":"This is a proposal issue for introducing DI container (Dependency Injection)\r\nFor diverse opinions, please feel free to leave any comment, @amiq11. \r\n\r\n## TL;DR\r\n\r\n* I hate the current structure of ``...ActivityBase`` and ``Flavor...Activity``\r\n* I want to restructure them by splitting ``...ActivityBase`` into common logic and flavor-specific logic\r\n* I want to introduce Dagger2\r\n\r\n## Background\r\nCurrently we are going to implement [product flavors](https://developer.android.com/studio/build/build-variants.html#product-flavors).\r\nThere are two main features of product flavors: determining java files and merging xml.\r\n\r\n(You can skip below 2 sections if you know product flavors)\r\n### determining java files\r\nWith product flavors, you can change files (or classes) by placing corresponding directories;\r\nsuppose you define 2 flavors (``flavor1``, ``flavor2``) and have ``flavor1/java/com/example/Test.java``, and ``flavor2/java/com/example/Test.java``,\r\nand the code in ``main/java/com/example/Main.java`` refers to ``Test`` class,\r\nthen the ``Test`` class is referred with accordance of the product flavor in build-time.\r\n\r\n### merging xml\r\nYou can define ``AndroidManifest.xml`` files for each product flavor and one ``main``.\r\n``main`` does not belong to flavors, but it defines common resources (xml, java, images).\r\nIf you have ``AndroidManifest.xml`` in ``main`` and ``flavor1``, Android Studio merges the two xml files in build-time.\r\nWhy this is useful is that you can change resources according to the flavor.\r\nA launch activity is defined in the manifest xml file. So you can choose the launch activity\r\nby defining the manifest xml in the flavor.\r\n\r\n### Why we use it and How we used it\r\nWhy this feature is needed for us is because:\r\n* we have to support multiple types of robots, each of which needs slightly different code (e.g., types of block, types of communication methods)\r\n* but code-duplication has to be minimized\r\n\r\nThat's why we are going to use productFlavors.\r\nWe have ``nxt``, ``ev3``, and ``pile`` robots so we'll have 3 product flavors.\r\n\r\nIn our current code, we  we define ``TitleActivity`` for each flavor and specify launch activities in manifest xml files for each flavor.\r\nWith the xml files, the launch activity of each app can start properly.\r\nAnd for programming (block-manipulation) and execution (communication with robots) activities,\r\nwe define ``ProgrammingActivityBase`` and ``ExecutionActivityBase`` respectively,\r\nand then in each flavor these classes are inherited according to the flavor (e.g., for ``nxt``, ``NxtProgrammingActivity`` and ``NxtExecutioinActivity``).\r\n\r\nThe code of ``ProgrammingActivityBase`` is like below:\r\n```java\r\npublic abstract class ProgrammingActivityBase extends Activity {\r\n\r\n    /* these methods are implemented in concrete classes */\r\n    protected abstract Intent getIntentForBlockList();\r\n\r\n    protected abstract Intent getIntentForExecution();\r\n\r\n    /* the rest is common for all flavors */\r\n    private void goToExecution() {\r\n        Intent intent = getIntetnForExecution();\r\n        ...\r\n    }\r\n    ...\r\n};\r\n```\r\n\r\nThen in ``NxtProgrammingActivity`` is like below:\r\n\r\n```java\r\npublic class NxtProgrammingActivity extends ProgrammingActivityBase {\r\n\r\n    @Override\r\n    protected Intent getIntentForBlockList() {\r\n        return new NxtBlockIntent();\r\n    }\r\n\r\n    @Override\r\n    protected Intent getIntentForExecution() {\r\n        return new NxtExecutionIntent();\r\n    }\r\n}\r\n```\r\n\r\n## Problem Statement\r\nWhat I am concering is that the structure seems awkward and the base classes are too huge for abstract class.\r\nIn my opinion, the straightforward way for this situation is that defining interface for flavor-specific logic (intent-provider for programming)\r\nand injecting flavor-specific-implementation into the interface.\r\n\r\nSo what I want to suggest is like below:\r\n\r\n```java\r\npublic class ProgrammingActivity extends Activity {\r\n    private IntentProvider intentProvider;\r\n\r\n    private void goToExecution() {\r\n        Intent intent = intentProvider.forExecution();\r\n        ...\r\n    }\r\n}\r\n\r\npublic interface IntentProvider {\r\n    Intent forExecution();\r\n}\r\n\r\npublic class NxtIntentProvider {\r\n    @Override\r\n    public Intent forExecution() {\r\n        return ...\r\n    }\r\n}\r\n```\r\n\r\nBy splitting code like above, the code gets\r\n* better organized\r\n* easier to be mocked\r\n* easier for new-comers to understand what is needed to be implemented for other flavors.\r\n\r\nAnd this proposal also solves #31.\r\n\r\n### Coping with dependency by DI container\r\nIf you feel my proposal sounds good, you might wonder how the interfaces are ``injected``.\r\nThat kind of problems are called Dependency Injection.\r\nThere are a number of pages describing this problem, so please google it.\r\n\r\nFor Android projects, there is a cool DI container called [Dagger2](http://google.github.io/dagger/).\r\nSo I want to use this for our android project.\r\n\r\nI tested the feasibility of my proposal in [here](https://github.com/myusak/DaggerTest)\r\n\r\nBy using Dagger2, the code would be like below:\r\n```java\r\n\r\npublic class ProgrammingActivity extends Activity {\r\n\r\n    @Inject\r\n    IntentProvider intentProvider;\r\n\r\n    /* same as above */\r\n}\r\n```\r\n\r\nHow cool, isn't it?\r\nThank you for reading this too long issue.\r\n"}}
{"actor":{"display_login":"myusak"}}