之前做过一段时刻团队 CR Owner 机制的落地，以及 CR 气氛和文化的提升，关于 CR 逐步有了一些更深的理解以及可落地的计划
个人理解，Code Review 是为了找出代码中「抱负」和「现实」之间的距离，所以怎么把 CR 做好，其实就能够拆解成两个问题
- 抱负的代码究竟是怎样的，也便是所谓的最佳实践
- 怎么找出代码中抱负和现实的距离，我给出的答案是从日常的 CR 活动中构成一份 CR 事例集
所以便有了这篇文章，期望从往常的 CR 活动中搜集最常见问题和改善计划，以及 Android 中可落地的最佳实践，构成一份极佳的 CR 事例集供开发者和 reviewer 参考，并给新同学一些指引和学习
一、CR 中常见的问题
1、 代码标准
主张阅读：Java 编码标准
1. 代码之间没有恰当的空格
代码之间需求有恰当的空格，看来也更加舒服，主张写完随手格局化一下
// Don't
public static void startIMMessageListActivity(Context context){
if (context!= null){
Intent intent =new Intent(context, IMMessageListActivity.class);
PluginLoader.getInstance().startPluginActivity(context,enterCallback);
}
}
// Do
public static void startIMMessageListActivity(Context context) {
if (context != null) {
Intent intent = new Intent(context, IMMessageListActivity.class);
PluginLoader.getInstance().startPluginActivity(context, enterCallback);
}
}
2. 运用魔法数
魔法数字（魔法数值）是代码中未经预先定义而直接呈现的数值 （1）尽量防止运用魔法数字，应代之有姓名的常量或枚举 （2）原则上代码中直接呈现的数值便是魔法数字， 经常被用作下标和初始值的 0 在外 （3）制止呈现相同数值的魔法数字两次
// Don't
private fun initLoadingView() {
with(binding.qqGroupLoadingView) {
if (qqGroupSize > 4) {
layoutParams.height = DensityUtils.dp2px(context, 277f)
} else {
val height = qqGroupSize * 65f
layoutParams.height = DensityUtils.dp2px(context, height)
}
this.layoutParams = layoutParams
}
}
// Do
private fun initLoadingView() {
with(binding.qqGroupLoadingView) {
if (qqGroupSize > THRESHOLD_QQ_GROUP_LIST) {
layoutParams.height = DensityUtils.dp2px(context, MAX_HEIGHT_QQ_GROUP_LIST)
} else {
val height = qqGroupSize * HEIGHT_QQ_GROUP_ITEM
layoutParams.height = DensityUtils.dp2px(context, height)
}
this.layoutParams = layoutParams
}
}
3. 大块的注释代码
抛弃代码主张直接删除去，后续想找回，回溯 Git 的 history 即可
// Don't
fun onPersonProfileEvent(event: PersonProfileEvent) {
when (event.code) {
PersonProfileEvent.EVENT_UPDATE_QQ_GROUP -> refresh()
// PersonProfileEvent.EVENT_UPDATE_QQ_GROUP_NAME -> {
// joinSlogan = event.qqGroupName
// setQQGroupIntroduction()
// }
PersonProfileEvent.EVENT_TOGGLE_QQ_GROUP_LIST -> {
dismissAllowingStateLoss()
}
}
}
// Do
fun onPersonProfileEvent(event: PersonProfileEvent) {
when (event.code) {
PersonProfileEvent.EVENT_UPDATE_QQ_GROUP -> refresh()
PersonProfileEvent.EVENT_TOGGLE_QQ_GROUP_LIST -> {
dismissAllowingStateLoss()
}
}
}
4. 代码中存在大量的 warning
代码开发完成后，主张 check 下增量代码中所有的 warning，尽量做到 0 warning
// Don't
android:layout_marginLeft="22dp"
android:layout_marginRight="15dp"
moduleDirector.getSubModuleVideoSelector().setOnBackListener(new OnBackListener() {
@Override
public void onBackClick() {
moduleDirector.openCollectionVideo(getContext(), getCurrentViewHolder());
}
});
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append("mInsertFeed.id=" + mInsertFeed.id);
stringBuilder.append(", mInsertFeed.feed_desc=" + mInsertFeed.feed_desc);
// Do
android:layout_marginStart="22dp"
android:layout_marginEnd="15dp"
moduleDirector.getSubModuleVideoSelector().setOnBackListener(() ->
moduleDirector.openCollectionVideo(getContext(), getCurrentViewHolder()));
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append("mInsertFeed.id=").append(mInsertFeed.id)
.append(", mInsertFeed.feed_desc=").append(mInsertFeed.feed_desc)
2、 业务逻辑
1. 反常逻辑没有处理
反常逻辑主张添加日志，便利后续定位问题，或许对反常逻辑进行上报，观察问题的数量级
// Don't
private fun onStartProfileActivity(personId: String?) {
if (personId.isNullOrEmpty()) {
return
}
...
}
private fun onCreate() {
if (Router.getService(LoginService.class).getCurrentUser() == null ) {
return;
}
...
}
// Do
private fun onStartProfileActivity(personId: String?) {
if (personId.isNullOrEmpty()) {
Logger.i(TAG, "personId is null or empty")
return
}
...
}
private fun onCreate(Bundle savedInstanceState) {
if (Router.getService(LoginService.class).getCurrentUser() == null) {
WSErrorReporter.reportError(module, subModule, errorName);
return;
}
...
}
2. 重复造轮子
大部分的工具类端内根本都有，开发需求之前主张先查找一波，直接运用或许进行拓宽
// Don't
class Utils {
public int dp2px(Float dp) {
return (int) (dp * sDensity);
}
}
class DisplayUtils {
public float dpToPx(Context context, float dp) {
float density = context.getResources().getDisplayMetrics().density;
return dp * density;
}
}
class ViewUtils {
public static int dpToPx(float dp) {
return DensityUtils.dp2px(GlobalContext.getContext(), dp);
}
}
// Do
class DensityUtils {
public static int dp2px(Context context, float dpVal) {
return (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, dpVal,
context.getResources().getDisplayMetrics());
}
}
3、 功能影响
1. entryKey 进行遍历
假如需求对 map 进行遍历并获取 value，主张直接经过 map.entries，而不是获取 map.keys 之后，再遍历获取 value
// Don't
val map = mapOf< String, String>()
val keySet = map.keys
for (key in keySet) {
Log.i(TAG, "value: ${map[key]}")
}
// Do
val map = mapOf< String, String>()
for (entry in map.entries) {
Log.i(TAG, "value: ${entry.value}")
}
2. 运用 ?. 代替 !!
在 Kotlin 中尽量少运用 !!，主张能够用 ?. 防止空指针反常
// Don't
ivAvatar = getChildView("single_feed_iv_avatar")!!.viewNative as AvatarViewV2
tvName = getChildView("single_feed_tv_name")!!.viewNative as TextView
// Do
ivAvatar = getChildView("single_feed_iv_avatar")?.viewNative as? AvatarViewV2
tvName = getChildView("single_feed_tv_name")?.viewNative as? TextView
3. 频繁的进行日志打印
尽管进行日志打印是个好习惯，频繁的进行日志打印则会影响 App 的流畅度
// Don't
@Override
fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) {
val layoutManager = recyclerView.layoutManager
Log.i(RecommendPageFragment.TAG, "onScrolled: dx $dx dy $dy")
}
4. 直接 import *
不要呈现相似这样的 import 句子：import java.util.* ，坚持 import 的整洁并尽或许防止歧义
// Don't
import android.os.*
// Do
import android.os.Bundle;
import android.os.Message;
4、 单测相关
1. 没有进行标准命名
- 测验类命名：ClassNameTest
- 测验方法命名：testClassMethodName_CaseName
// Don't
class TransferMonitor {
@Test
fun needNetProbe_when_network_error() {}
}
// Do
class TransferMonitorTest {
@Test
fun testNeedNetProbe_WhenNetworkError() {}
}
2. mock 之后，没有在 @AfterAll 中进行 unmock
// Don't
@AfterAll
fun tearDown() {
// nothing
}
// Do
@AfterAll
fun tearDown() {
unmockkAll()
Router.unregisterAllService()
}
3. 运用 Kotlin assert 或 Junit4 / 5 assert 进行测验
单元测验，主张一致运用 Kotlin + Junit 5 + Truth，代码简练、可读性高并且运行速度快
- Kotlin assert：接口单一、失利信息可读性差
- Junit4 / 5 assert：接口运用不清晰、失利信息可读性一般、易误解
- Truth：接口丰厚、一致性高、失利信息可读性高
// Don't
val actual = "foo"
assert(actual == "bar")
val actual = "foo"
Assertions.assertEquals(actual, "bar")
// Do
val actual = "foo"
Truth.assertThat(actual).hasLength(3)
4. 测验用例里边测验多种条件
每个测验用例只测一种条件，假如有比较多的 case，主张运用分组测验、参数化测验
// Don't
@Test
fun testNeedNetProbe() {
var task = TransferMonitorTask(1, "cmd", 10, Job())
task.addStage(TransferStageFlag.STAGE_TRANSFER_START, System.currentTimeMillis())
assertTrue(monitor.needNetProbe(task, false))
assertTrue(!monitor.needNetProbe(task, true))
}
// Do
@Nested
inner class NeedNetProbe {
@Test
fun testNeedNetProbe_WhenNetworkError() {}
@Test
fun testNeedNetProbe_WhenNonNetworkError() {}
@Test
fun testNeedNetProbe_WhenNetworkTakeHugeTime() {}
}
5. 运用接口隔离依靠接口而不是详细的类
运用接口隔离能够使咱们的代码可测性更强，并且有效减少 mock，降低单测耗时
// Don't
public WnsEnvironmentSubServiceImpl(WnsClient wnsClient) {
mWnsClient = wnsClient;
}
@Test
fun testGetIpString() {
val sp = mock<SharedPreferences>().apply {
every { edit() } returns mockk()
}
mockkStatic(Global::class)
every { Global.currentProcessName() } retusn ""
every { Global.getSharedPreferences(any(), any()) } returns sp
val impl = WnsEnvironmentSubServiceImpl(WnsClient(Client()))
...
}
// Do
public WnsEnvironmentSubServiceImpl(IWnsClientWrapper wnsClientWrapper) {
mWnsClient = wnsClient;
}
class WnsClientWrapperStub : IWnsClientWrapper {
...
}
@Test
fun testGetIpString() {
val impl = WnsEnvironmentSubServiceImpl(WnsClientWrapperStub())
...
}
二、Android 最佳实践
1、反常处理
1. 【强制】能够经过预检查躲避的 RuntimeException 不应该经过 catch 方法来处理
例如，NullPointerException，IndexOutOfBoundsException 不要用 try catch 来进行处理。无法经过预检查的反常在外，比如，在解析字符串形式的数字时，或许存在数字格局错误，不得不经过 catch NumberFormatException 来完成。
// Don't
try {
obj.method();
} catch (NullPointerException e) {
...
}
// Do
f (obj != null) {
...
}
2. 【强制】反常不能用于流程控制
不主张运用反常作为流程控制的原因有两点：
① 影响函数的易用性 反例：运用中台播放器进行 seek 的时分，播放器对当前的状态机进行了校验，假如不符合预期，直接抛出了反常，这种处理计划看起来也比较合理，进行了严格的状态校验，可是过于生硬了，在 crash 与 seek 失利两种情况下，显着 crash 的结果要严峻的多。并且此刻 seek 失利或许是用户无感知的。所以比较推荐的方法，是打印 seek 失利日志，然后进行 return。
@Override
public void seekTo(int positionMs) throws IllegalStateException {
TPLogUtil.i(TAG, "seekTo:" + positionMs);
throwExceptionIfPlayerReleased();
int ret = mPlayer.seekToAsync(positionMs, TPNativePlayerSeekMode.PREVIOUS_KEY_FRAME, 0);
if (ret != TPGeneralError.OK) {
throw new IllegalStateException("seek to position:" + positionMs + " failed!!");
}
}
② 功率低 反常处理的功率会远低于条件判断，运用小米 10Pro 进行测验，正例的时刻消费大约在 0-1ms，反例的时刻消费大约在 44-50ms。
// Don't
private void handleOnClickTryCatch() {
for (int i = 0; i < 10000; i++) {
try {
seekTo(0);
} catch (Exception e) {
//ignore，防止影响功能，对测验产生搅扰
}
}
}
private void seekTo(int pos) throws Exception {
throw new Exception();
}
// Do
private void handleOnClickCondition() {
for (int i = 0; i < 10000; i++) {
seekToNoEx(0);
}
}
private void seekToNoEx(int pos) {
currentPos = pos;
}
3. 【强制】不要对⼤段代码进⾏ try catch
对大段代码进行 try-catch 程序无法根据不同的反常做出正确的应激反响，也不利于定位问题，这是一种不负责任的体现。
4. 【强制】反常捕获有必要处理
5. 【强制】不要在 fina中 运用 return
try 块中的 return 句子成功后，并不立刻回来，而是持续执行 finally 块中的句子，假如此处存在 return 句子，则在此直接回来，无情丢弃掉 try 块中的回来点。
// Don't
private int x = 0;
public int checkReturn() {
try {
// x 等于 1，此处不回来
return ++x;
} finally {
// 回来的结果是 2
return ++x;
}
}
6. 【强制】finally 中有必要对资源进⾏开释
在 finally 中开释资源时，能够运用函数封装优雅。并且关于嵌套流，不必层层关闭，只需关闭最外层的流。Exception 不要运用 print StackTrace 直接输出，运用 log 进行封装，最好标记这个 Exception 是已经捕获的。
// Do
private User readUser() {
FileInputStream fileStream = null;
ObjectInputStream input = null;
User user = null;
try {
fileStream = new FileInputStream("Object.txt");
input = new ObjectInputStream(fileStream);
user = (User) input.readObject();
} catch (Exception e) {
logger.info("exception catched:" + Log.getStackTraceString(e));
} finally {
closeSafe(input);
}
return user;
}
假如 JDK7 及以上，能够运用 try-witesources。AutoCloseable 需求继承 AutoCloseable。
// Do
try(Resource resource = new Resource()) {
resource.sayHello();
} catch (Exception e) {
e.printStackTrace();
}
2、插件开发
1. 插件中不要引⽤主⼯程中的 final 变量
除非你确定它不会变化，由于在插件编译时这个值就会被固定，并不会跟着主工程中该final变量值的更改而变化。
反例：
3、安全规约
1. 用户敏感数据制止直接展现，有必要对展现数据进⾏脱敏。
阐明：中国大陆个人手机号码显现为：158****9119，中间 4 位，防止隐私走漏。
2. 尽量使组件制止外部拜访
当 Android 四大组件不需其他应用拜访时，显现注明 android:exported=false，由于 exported 的默认值或许发生变化。 当组件包含 时，exported 默以为 true，否则默以为 false。
3. 防止运用全局播送传递敏感信息
能够运用 LocalBrdcastManager 进行代替，LocalBroadcastManager 根据 Handler，具有更好的功率，可是只能在同进程内传递。 关于运用全局播送，能够经过 Intent.setPackage 来约束接收方包名，来保证安全。 然而为难的是 LocalBroadcastManager 在新的版本中已经抛弃，取而代之的是 LiveData 和 Reactive streams。用法后续更新…
4、进程相关
1. Binder 传输数据大小约束为 1M
所以根据 Binder 的通讯方法都会收到此约束，例如运用 Intent 在组件中传递数据。
2. 制止运用 New Thread 方法创立线程
由于会有显着的延迟，⼤量使⽤后会对体系功能形成额定的开支。
3. 运用播送跨进城通讯时，防止呈现播送震动
使⽤名为 caller 的 int 值来表示发动类型，存在多个进程中，当值发⽣变化时，告诉其他进程跟从变化。当 caller 值在两个进程中一起变化时，就或许触发⼴播震动，产⽣死循环。
解决计划： 运用时刻戳来表示最近的一次修改，或许运用 ContentProvider 来进行值的跨进程传输。
5、功能优化
1. 合理运用 LAYER_TYPE_HARDE 提高动画功能
经过 View.setLayerType 接⼝ View 的制作⽅式，默认值是 LAYER_TYPE_NONE。 假如设置参数为 LAYER_TYPE_HARDWARE，并且翻开硬件加速，就会产⽣离屏缓冲，若 View 内部元素不更新，这时对 View 做动画功率会⾼很多，例如桌⾯左右翻屏时。 LAYER_TYPE_SOFTWAR E会将 View 制作到 Bitmap 中，一般不会运用。
2. 运用 Printer 监控线程卡顿
使⽤ Android 现有的机制 Printer，在 Looper 执⾏单个使命前后打印，就能够知道使命的执⾏时刻，咱们设置⼀个阈值，然后打印线程堆栈，就知道哪个使命卡顿了。
/**
* Run the message queue in this thread. Be sure to call * {@link #quit()} to end the loop.
*/
public static void loop() {
...
for (; ; ) {
Message msg = queue.next(); // might block ...
// This must be in a local variable, in case a UI event sets the logger
final Printer logging = me.mLogging;
if (logging != null) {
logging.println(">>>>> Dispatching to " + msg.target + " " + msg.callback + ": " + msg.what); }
...
try {
msg.target.dispatchMessage(msg);
dispatchEnd = needEndTime ? SystemClock.uptimeMillis() : 0;
} finally {
if (traceTag != 0) {
Trace.traceEnd(traceTag);
}
}
...
if (logging != null) {
logging.println("<<<<< Finished to " + msg.target + " " + msg.callback);
}
...
}
}
3. 不要运用 SharePreference 进行跨进程通讯
尽管 Google 提供了 MODE_MULTI_PROCESS 模式，可是其原理只是在 getSharedPreferences 时，重新加载了 xml，所以功能很差，跨进程数据传输请使⽤ ContentProvider。
@Override
public SharedPreferences getSharedPreferences(String name, int mode) {
SharedPreferencesImpl sp;
...
if ((mode & Context.MODE_MULTI_PROCESS) != 0 || getApplicationInfo().targetSdkVersion < android.os.Build.VERSION_CODES.HONEYCOMB) {
// If somebody else (some other process) changed the prefs
// file behind our back, we reload it. This has been the
// historical (if undocumented) behavior.
sp.startReloadIfChangedUnexpectedly();
}
return sp;
}
4. 序列化场景最好运用 FlatBuffer
FlatBuffers 是⼀个开源的、跨平台的、⾼效的、提供了 C++/Java 接⼝的序列化⼯具库。 它是 Google 专⻔为游戏开发或其他功能敏感的应⽤程序需求⽽创立的。特别适⽤于移动平台。
首要优点：
● 对序列化数据的拜访不需求打包和拆包，它将序列化数据存储在缓存中，这些数据既 可存储在文件中，又能够经过网络原样传输，而没有任何解析开支； ● 内存功率和速度：拜访数据时的唯一内存需求便是缓冲区，不需求额定的内存分配。 这可检查详细的基准测验。 ● 扩展性、灵活性：它支撑的可选字段意味着不仅能获得很好的前向/后向兼容性（对 于生命周期的游戏来说特别重要，由于不需求每个新版本都更新所有数据）。 ● 最小代码依靠：仅仅需求自动生成的少量代码和一个单一的头文件依靠，很容易集成 到有体系中。 ● 强类型规划：尽或许使错误呈现在编译期，而不是比及运行期才手动检查和批改。 ● 运用简略：生成的 C++ 代码提供了简略的拜访和结构接口；并且假如需求，经过一个可选功能能够在运行时，高效解析 Schema 和类 JSON 格局的文本。 ● 跨平台：支撑 C++11、Java，而不需求任何依靠库；在最新的 gcc、clng、vs2010 等编译器上作业良好。
