最近兩個月一直在做團隊 CR Owner 機制的落地，以及 CR 氛圍和文化的提升，對于 CR 逐漸有了一些更深的理解以及可落地的方案
個人理解，Code Review 是為了找出代碼中「理想」和「現(xiàn)實」之間的差距，所以如何把 CR 做好，其實就可以拆解成兩個問題
- 理想的代碼究竟是怎樣的，也就是所謂的最佳實踐
- 如何找出代碼中理想和現(xiàn)實的差距，我給出的答案是從日常的 CR 活動中形成一份 CR 案例集
于是便有了這篇文章，希望從平常的 CR 活動中收集最常見問題和改進方案，以及 Android 中可落地的最佳實踐，形成一份極佳的 CR 案例集供開發(fā)者和 reviewer 參考，并給新同學一些指引和借鑒
一、CR 中常見的問題
1、 代碼規(guī)范
建議閱讀：Java 編碼規(guī)范
1. 代碼之間沒有適當?shù)目崭?/h4>
代碼之間需要有適當?shù)目崭?，看來也更加舒服，建議寫完隨手格式化一下
// 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. 使用魔法數(shù)
魔法數(shù)字（魔法數(shù)值）是代碼中未經預先定義而直接出現(xiàn)的數(shù)值
（1）盡量避免使用魔法數(shù)字，應代之有名字的常量或枚舉
（2）原則上代碼中直接出現(xiàn)的數(shù)值就是魔法數(shù)字， 經常被用作下標和初始值的 0 除外
（3）禁止出現(xiàn)相同數(shù)值的魔法數(shù)字兩次
// 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. 大塊的注釋代碼
廢棄代碼建議直接刪除掉，后續(xù)想找回，回溯 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
代碼開發(fā)完成后，建議 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、 業(yè)務邏輯
1. 異常邏輯沒有處理
異常邏輯建議增加日志，方便后續(xù)定位問題，或者對異常邏輯進行上報，觀察問題的數(shù)量級
// 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. 重復造輪子
大部分的工具類端內基本都有，開發(fā)需求之前建議先搜索一波，直接使用或者進行拓展
// 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 *
不要出現(xiàn)類似這樣的 import 語句：import java.util.* ，保持 import 的整潔并盡可能避免歧義
// Don't
import android.os.*
// Do
import android.os.Bundle;
import android.os.Message;
4、 單測相關
1. 沒有進行規(guī)范命名
- 測試類命名：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 進行測試
單元測試，建議統(tǒng)一使用 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，建議使用分組測試、參數(shù)化測試
// 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. 【強制】可以通過預檢查規(guī)避的 RuntimeException 不應該通過 catch 方式來處理
例如，NullPointerException，IndexOutOfBoundsException 不要用 try catch 來進行處理。無法通過預檢查的異常除外，比如，在解析字符串形式的數(shù)字時，可能存在數(shù)字格式錯誤，不得不通過 catch NumberFormatException 來實現(xiàn)。
// Don't
try {
obj.method();
} catch (NullPointerException e) {
...
}
// Do
f (obj != null) {
...
}
2. 【強制】異常不能用于流程控制
不建議使用異常作為流程控制的原因有兩點：
① 影響函數(shù)的易用性
反例：使用中臺播放器進行 seek 的時候，播放器對當前的狀態(tài)機進行了校驗，如果不符合預期，直接拋出了異常，這種處理方案看起來也比較合理，進行了嚴格的狀態(tài)校驗，但是過于生硬了，在 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 程序無法根據(jù)不同的異常做出正確的應激反應，也不利于定位問題，這是一種不負責任的表現(xiàn)。
4. 【強制】異常捕獲必須處理
5. 【強制】不要在 fina中 使用 return
try 塊中的 return 語句成功后，并不馬上返回，而是繼續(xù)執(zhí)行 finally 塊中的語句，如果此處存在 return 語句，則在此直接返回，無情丟棄掉 try 塊中的返回點。
// Don't
private int x = 0;
public int checkReturn() {
try {
// x 等于 1，此處不返回
return ++x;
} finally {
// 返回的結果是 2
return ++x;
}
}
6. 【強制】?nally 中必須對資源進?釋放
在 finally 中釋放資源時，可以使用函數(shù)封裝優(yōu)雅。并且對于嵌套流，不必層層關閉，只需關閉最外層的流。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、插件開發(fā)
1. 插件中不要引?主?程中的 ?nal 變量
除非你確定它不會變化，因為在插件編譯時這個值就會被固定，并不會隨著主工程中該final變量值的更改而變化。
反例：
在插件中希望能獲取 GlobalConfig.SDK_VERSION 這個值，這塊在編譯的時候會被直接賦予一個固定的值，并不會隨著主工程變量值的更改而變化。我們反編譯后可以發(fā)現(xiàn)
3、安全規(guī)約
1. 用戶敏感數(shù)據(jù)禁止直接展示，必須對展示數(shù)據(jù)進?脫敏。
說明：中國大陸個人手機號碼顯示為：158****9119，中間 4 位，防止隱私泄露。
2. 盡量使組件禁止外部訪問
當 Android 四大組件不需其他應用訪問時，顯示注明 android:exported=false，因為 exported 的默認值可能發(fā)生變化。
當組件包含 <intent-filter> 時，exported 默認為 true，否則默認為 false。
3. 避免使用全局廣播傳遞敏感信息
可以使用 LocalBrdcastManager 進行替代，LocalBroadcastManager 基于 Handler，擁有更好的效率，但是只能在同進程內傳遞。
對于使用全局廣播，可以通過 Intent.setPackage 來限制接收方包名，來保證安全。
然而尷尬的是 LocalBroadcastManager 在新的版本中已經廢棄，取而代之的是 LiveData 和 Reactive streams。用法后續(xù)更新...
4、進程相關
1. Binder 傳輸數(shù)據(jù)大小限制為 1M
所以基于 Binder 的通信方式都會收到此限制，例如使用 Intent 在組件中傳遞數(shù)據(jù)。
2. 禁止使用 New Thread 方式創(chuàng)建線程
因為會有明顯的延遲，?量使?后會對系統(tǒng)性能造成額外的開銷。
3. 使用廣播跨進城通信時，防止出現(xiàn)廣播震蕩
使?名為 caller 的 int 值來表示啟動類型，存在多個進程中，當值發(fā)?變化時，通知其他進程跟隨變化。當 caller 值在兩個進程中同時變化時，就可能觸發(fā)?播震蕩，產?死循環(huán)。
解決方案：
使用時間戳來表示最近的一次修改，或者使用 ContentProvider 來進行值的跨進程傳輸。
5、性能優(yōu)化
1. 合理使用 LAYER_TYPE_HARDE 提高動畫性能
通過 View.setLayerType 接? View 的繪制?式，默認值是 LAYER_TYPE_NONE。 如果設置參數(shù)為 LAYER_TYPE_HARDWARE，并且打開硬件加速，就會產?離屏緩沖，若 View 內部元素不更新，這時對 View 做動畫效率會?很多，例如桌?左右翻屏時。
LAYER_TYPE_SOFTWAR E會將 View 繪制到 Bitmap 中，一般不會使用。
2. 使用 Printer 監(jiān)控線程卡頓
使? Android 現(xiàn)有的機制 Printer，在 Looper 執(zhí)?單個任務前后打印，就可以知道任務的執(zhí)?時間，我們設置?個閾值，然后打印線程堆棧，就知道哪個任務卡頓了。
/**
* 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，所以性能很差，跨進程數(shù)據(jù)傳輸請使? 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 專?為游戲開發(fā)或其他性能敏感的應?程序需求?創(chuàng)建的。尤其適?于移動平臺。
主要優(yōu)點：
● 對序列化數(shù)據(jù)的訪問不需要打包和拆包，它將序列化數(shù)據(jù)存儲在緩存中，這些數(shù)據(jù)既
可存儲在文件中，又可以通過網絡原樣傳輸，而沒有任何解析開銷；
● 內存效率和速度：訪問數(shù)據(jù)時的唯一內存需求就是緩沖區(qū)，不需要額外的內存分配。
這可查看詳細的基準測試。
● 擴展性、靈活性：它支持的可選字段意味著不僅能獲得很好的前向/后向兼容性（對
于生命周期的游戲來說尤其重要，因為不需要每個新版本都更新所有數(shù)據(jù)）。
● 最小代碼依賴：僅僅需要自動生成的少量代碼和一個單一的頭文件依賴，很容易集成
到有系統(tǒng)中。
● 強類型設計：盡可能使錯誤出現(xiàn)在編譯期，而不是等到運行期才手動檢查和修正。
● 使用簡單：生成的 C++ 代碼提供了簡單的訪問和構造接口；而且如果需要，通過一個可選功能可以在運行時，高效解析 Schema 和類 JSON 格式的文本。
● 跨平臺：支持 C++11、Java，而不需要任何依賴庫；在最新的 gcc、clng、vs2010 等編譯器上工作良好。
