{"action":"created","issue":{"url":"https://api.github.com/repos/open-keychain/open-keychain/issues/1603","labels_url":"https://api.github.com/repos/open-keychain/open-keychain/issues/1603/labels{/name}","comments_url":"https://api.github.com/repos/open-keychain/open-keychain/issues/1603/comments","events_url":"https://api.github.com/repos/open-keychain/open-keychain/issues/1603/events","html_url":"https://github.com/open-keychain/open-keychain/pull/1603","id":118236866,"number":1603,"title":"MVP Pattern (for decrypt list activity)","user":{"login":"Valodim","id":27813,"avatar_url":"https://avatars.githubusercontent.com/u/27813?v=3","gravatar_id":"","url":"https://api.github.com/users/Valodim","html_url":"https://github.com/Valodim","followers_url":"https://api.github.com/users/Valodim/followers","following_url":"https://api.github.com/users/Valodim/following{/other_user}","gists_url":"https://api.github.com/users/Valodim/gists{/gist_id}","starred_url":"https://api.github.com/users/Valodim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Valodim/subscriptions","organizations_url":"https://api.github.com/users/Valodim/orgs","repos_url":"https://api.github.com/users/Valodim/repos","events_url":"https://api.github.com/users/Valodim/events{/privacy}","received_events_url":"https://api.github.com/users/Valodim/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/open-keychain/open-keychain/labels/in%20progress","name":"in progress","color":"ededed"}],"state":"open","locked":false,"assignee":{"login":"Valodim","id":27813,"avatar_url":"https://avatars.githubusercontent.com/u/27813?v=3","gravatar_id":"","url":"https://api.github.com/users/Valodim","html_url":"https://github.com/Valodim","followers_url":"https://api.github.com/users/Valodim/followers","following_url":"https://api.github.com/users/Valodim/following{/other_user}","gists_url":"https://api.github.com/users/Valodim/gists{/gist_id}","starred_url":"https://api.github.com/users/Valodim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Valodim/subscriptions","organizations_url":"https://api.github.com/users/Valodim/orgs","repos_url":"https://api.github.com/users/Valodim/repos","events_url":"https://api.github.com/users/Valodim/events{/privacy}","received_events_url":"https://api.github.com/users/Valodim/received_events","type":"User","site_admin":false},"milestone":null,"comments":6,"created_at":"2015-11-22T03:41:52Z","updated_at":"2015-11-24T15:21:06Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/open-keychain/open-keychain/pulls/1603","html_url":"https://github.com/open-keychain/open-keychain/pull/1603","diff_url":"https://github.com/open-keychain/open-keychain/pull/1603.diff","patch_url":"https://github.com/open-keychain/open-keychain/pull/1603.patch"},"body":"Soo~ I played around a bit with the Model-View-Presenter (MVP) pattern, implementing it in the DecryptActivity. The basic idea is to reduce the displaying component (i.e. Fragment in our cases) to a purely passive view, and move all logic into a Presenter class. Communication happens with raw events from the ui to the presenter (like onUiLongClickFile), and updates to the ui state from the presenter to the ui (like displayFileContextMenu).\r\n\r\nThere are some limitations to this approach due to basic Android design assumptions, the presenter in particular needs to move some things through the UI component which aren't strictly UI, like startActivity/onActivityResult.\r\n\r\nStill, I really like this approach. Makes it much easier to redesign or switch out UI components (and god knows we do that a lot), and also test ui logic independently from views themselves. It also makes lifecycle management a little easier, and makes the interface very clear - check out DecryptListInterface.java, it neatly ties logic and interface together.\r\n\r\nI'm not sure how much we gain from rewriting existing code to use MVP, but for me the experiment was quite successful, and I will probably use the pattern when I create new activities (of some complexity) in the future"},"comment":{"url":"https://api.github.com/repos/open-keychain/open-keychain/issues/comments/159302219","html_url":"https://github.com/open-keychain/open-keychain/pull/1603#issuecomment-159302219","issue_url":"https://api.github.com/repos/open-keychain/open-keychain/issues/1603","id":159302219,"user":{"login":"Valodim","id":27813,"avatar_url":"https://avatars.githubusercontent.com/u/27813?v=3","gravatar_id":"","url":"https://api.github.com/users/Valodim","html_url":"https://github.com/Valodim","followers_url":"https://api.github.com/users/Valodim/followers","following_url":"https://api.github.com/users/Valodim/following{/other_user}","gists_url":"https://api.github.com/users/Valodim/gists{/gist_id}","starred_url":"https://api.github.com/users/Valodim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Valodim/subscriptions","organizations_url":"https://api.github.com/users/Valodim/orgs","repos_url":"https://api.github.com/users/Valodim/repos","events_url":"https://api.github.com/users/Valodim/events{/privacy}","received_events_url":"https://api.github.com/users/Valodim/received_events","type":"User","site_admin":false},"created_at":"2015-11-24T15:21:06Z","updated_at":"2015-11-24T15:21:06Z","body":"If by \"postponing the discussion\" you mean \"don't use it for now\", then no, I would rather discuss this now as I don't see how the situation could be different in the next months.\r\n\r\nTestability increases, see cketti's commit. I don't agree with your view that MVP makes our code more difficult to read for potential contributors, but this is an argument which is hard to back with facts in either direction.\r\n\r\nThe decision here comes down to whether you somehow see the use of this pattern as so damaging to our codebase to veto against me using it. I'm not demanding we switch every activity to MVP, I don't plan to invest a huge amount of time into rewriting everything, nor do I request we make this a guideline contributors need to adhere to for their pull requests to be accepted. What I would want is that nobody actively changes the structure of activities back where I use MVP, and if at all possible keep new code in these activities compliant."}}