几点建议

[Piasy]

• fabric 的 api key 没有 ignore
• 依赖声明有多处没有指明版本(有 + 号)
• app/develop 目录是干嘛的?
• 是否可以整理一个架构 wiki?
• 都 finish 了, 还 startActivityForResult 干嘛?
• 为什么不是在 animation 结束的时候做跳转, 而是延迟固定的时间呢? 要是动画时长改了, 这里是不是也就要改了?
• AndroidStudio 2.2 beta 此处有两个 lint 提示, 好像有道理
• DCL 单例没用 volatile
• 关于 if 语句是否需要花括号的讨论, 有一点感觉有道理: 如果你需要扩展为多行语句, 要么有花括号, 要么单行, 否则容易疏忽出错
• 此处有 lint 提示, 建议升级 IDE, 并且对 lint 提示保持关注
• RxJava 避免内存泄漏
• 像这个这么复杂的逻辑, 就值得写个单元测试了
• 可以看看 MVP 模式?
• 可以考虑下依赖注入? 虽然你也有一个 Injection 类
• 为什么要有 RemoteTokenDataSource 这个类? 为了扩展 TokenContract.Remote (提供超出 API 范围的 data 接口)? 或者对 API 请求做些控制? 如果是前者, 我觉得 TokenRepository 做这个事情还挺合适, 如果是后者倒还是可以理解
• 另外, 有必要抽象一个 TokenRepository 接口吗? 虽说面向接口是很好的实践, 但如果不会有其他的实现, 接口确实增加了负担

最后有个问题想要一起讨论下

• 获取到新的 remote token 之后, 刷新 local token, TokenRepository 和 UserSession, 这件事谁做更合适?
• 更抽象一点，presenter 和 model，是否应该 model 提供基本、通用、业务无关的接口，由 presenter 利用业务无关接口实现业务相关需求？好吧，刚写出来我就觉得答案应该是肯定的。

[ryanhoo]

@Piasy 首先非常感谢这么用心的 review。

抱歉，因为处于内测的阶段，代码没有完全整理完毕，有几个是遗留问题，或者疏忽了。

fabric 的 api key 没有 ignore

• 疏忽，待改正。

这里是比较麻烦的一点，fabric 对 key 是严格验证的，我不能用 mock 的 key（无法通过 build: invalid key），采取的做法是在没有提供 fabric.properties 的时候，禁用 Fabric 功能。

都 finish 了, 还 startActivityForResult 干嘛?

• 老版本留下来的漏网之鱼，那个时候没有用 RxBus，用的是传统的方式判断是否登录成功。

依赖声明有多处没有指明版本(有 + 号)

• 理论上是不推荐用 + 号，防止版本变更，老版本 API 表现不一致或者 API 废弃，导致编译失败。
  而我用 + 号是因为，一般来说，版本号大概是这样 1.0.0，第三个版本一般是用来表达 bug 修复之类的变更，如果有功能、API 变更，应该反应在版本号的前两位上。因此我在使用 1.0.+ 这样的形式，希望能保持对 bug 修复版本的及时更新。
  当然这么做值得商榷。

app/develop 目录是干嘛的?

• 渠道专属目录，默认的渠道是 develop，我为这个渠道专门绘制了图标，而且使用不一样的 application id，目的是方便我区分测试和线上的应用。

是否可以整理一个架构 wiki?

• 可以，我还有计划就是为这个项目写一系列博客，本来就是技术的实验田，有很多值得分享、讨论的话题。

为什么不是在 animation 结束的时候做跳转, 而是延迟固定的时间呢? 要是动画时长改了, 这里是不是也就要改了?

• 历史遗留问题，待解决。

AndroidStudio 2.2 beta 此处有两个 lint 提示, 好像有道理

• 有道理

DCL 单例没用 volatile

• 补上，原来的单例不完全正确。

关于 if 语句是否需要花括号的讨论, 有一点感觉有道理: 如果你需要扩展为多行语句, 要么有花括号, 要么单行, 否则容易疏忽出错

• 因为懒而且确认只有一行代码，所以写了 Yoda Condition，应该一直有花括号。

此处有 lint 提示, 建议升级 IDE, 并且对 lint 提示保持关注

• 有理

RxJava 避免内存泄漏

• 漏了，用 MVP 重构的时候补上。

像这个这么复杂的逻辑, 就值得写个单元测试了

• 单元测试现在完全没有，没什么经验，希望你多多指导。 😆

可以看看 MVP 模式?

• 部分已经用 MVP 重构完了，这里还没有（内测嘛，嘿嘿嘿）。

可以考虑下依赖注入? 虽然你也有一个 Injection 类

• 计划用 Dagger 2，关注 README 的 Todo List，项目早期特意采用比较原始的方式实现，然后一步步重构来优化代码，目的是为了感受代码改变带来的变化，以及各种方式的优劣，另外还有一个目的就是为了向学习这个项目的新手做 demonstration。

为什么要有 RemoteTokenDataSource 这个类? 为了扩展 TokenContract.Remote (提供超出 API 范围的 data 接口)? 或者对 API 请求做些控制? 如果是前者, 我觉得 TokenRepository 做这个事情还挺合适, 如果是后者倒还是可以理解，另外, 有必要抽象一个 TokenRepository 接口吗? 虽说面向接口是很好的实践, 但如果不会有其他的实现, 接口确实增加了负担。

• 这个跟下面两个问题一样，处于大问号的阶段，需要寻找改进的方法。

获取到新的 remote token 之后, 刷新 local token, TokenRepository 和 UserSession, 这件事谁做更合适?
更抽象一点，presenter 和 model，是否应该 model 提供基本、通用、业务无关的接口，由 presenter 利用业务无关接口实现业务相关需求？好吧，刚写出来我就觉得答案应该是肯定的。