Android CR 案例集 & 最佳实践

作者: developerHaoz | 来源:发表于2020-12-09 19:45 被阅读0次

最近两个月一直在做团队 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. 【强制】ﬁnally 中必须对资源进⾏释放

在 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. 插件中不要引⽤主⼯程中的 ﬁnal 变量

除非你确定它不会变化，因为在插件编译时这个值就会被固定，并不会随着主工程中该final变量值的更改而变化。

反例：

在插件中希望能获取 GlobalConfig.SDK_VERSION 这个值，这块在编译的时候会被直接赋予一个固定的值，并不会随着主工程变量值的更改而变化。我们反编译后可以发现

3、安全规约

1. 用户敏感数据禁止直接展示，必须对展示数据进⾏脱敏。

说明：中国大陆个人手机号码显示为：158****9119，中间 4 位，防止隐私泄露。

2. 尽量使组件禁止外部访问

当 Android 四大组件不需其他应用访问时，显示注明 android:exported=false，因为 exported 的默认值可能发生变化。
当组件包含 <intent-filter> 时，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

FlatBuﬀers 是⼀个开源的、跨平台的、⾼效的、提供了 C++/Java 接⼝的序列化⼯具库。它是 Google 专⻔为游戏开发或其他性能敏感的应⽤程序需求⽽创建的。尤其适⽤于移动平台。

主要优点：

● 对序列化数据的访问不需要打包和拆包，它将序列化数据存储在缓存中，这些数据既
可存储在文件中，又可以通过网络原样传输，而没有任何解析开销；
● 内存效率和速度：访问数据时的唯一内存需求就是缓冲区，不需要额外的内存分配。
这可查看详细的基准测试。
● 扩展性、灵活性：它支持的可选字段意味着不仅能获得很好的前向/后向兼容性（对
于生命周期的游戏来说尤其重要，因为不需要每个新版本都更新所有数据）。
● 最小代码依赖：仅仅需要自动生成的少量代码和一个单一的头文件依赖，很容易集成
到有系统中。
● 强类型设计：尽可能使错误出现在编译期，而不是等到运行期才手动检查和修正。
● 使用简单：生成的 C++ 代码提供了简单的访问和构造接口；而且如果需要，通过一个可选功能可以在运行时，高效解析 Schema 和类 JSON 格式的文本。
● 跨平台：支持 C++11、Java，而不需要任何依赖库；在最新的 gcc、clng、vs2010 等编译器上工作良好。