工作中遇到的代码反例

前言

最近一直都是 copy 工程师和 scapegoat 工程师

不过也发现了一些些问题，也用来警示自己以后写代码 尽量 考虑全面(人无完人···)

反例1-下次进入 App，回到上次退出的页面

如下图

tab-example.jpg

类似知乎App的场景，顶部有一排 tab，每次记录选中的是哪一个 tab，下次进入时展示上次退出时的 tab。

原本的代码中是这样实现的(伪代码)

// 监听 ViewPager 回调
public void onPageSelected(int position) {
    // 1.拿当前 fragment
    fragment = getCurFragment();
    // 2.通过 fragment 拿 channel
    channel = getChannel(Fragment);
    // 3.保存 channel(int值)
    savePage(channel);
}

public void getChannel(Fragment fragment) {
   if (fragment instanceof FragmentA) {
       return 1;
   }
   // ···各个 tab
}

到这里也还好，但是当初始化的时候逻辑就很奇怪了

@Override
public void onViewCreated(View view, Bundle savedInstanceState) {
    // 1.获取之前保存在本地的 channel
    channel = getChannel();
    // 2.通过 channel 获取 position（我第一次看的时候是懵逼的，凭啥？）
    position = getPosition(channel);
    setInitTabPosition(position);
}

public int getPosition(int channel) {
    // 只为了表示 channel 和 position 有映射关系
    switch(channel) {
        case 5:
            return 0;
        case 7:
            return 1;
    }
}

这段代码就是想从本地取之前存好的 channel(也就是代表哪个 Fragment)，然后通过 channel 和 position 的映射关系拿到 position，传给 ViewPager 从而让此 Fragment 对应的 tab 变为选中状态。

但是，这段代码完全没有考虑我这个后来者。如果 Fragment 的顺序永远固定，那么也就算了，但是显然是不可能的···

所以，一旦 Fragment 的数量或者位置可以主动变化，那么这段代码就没法维护了，全部乱套。

拿上面的知乎为例，假设我停在了 热榜 页面，此时我退出了 app，存储 channel = 热榜，channel 和 position 的映射关系为 channel = 热榜 & position = 2。这时候我又进入了 app，如果 tab 是动态下发的，那么热榜前面可能多了两个 tab，此时顺序完全错乱。又或者我处于登录状态，然后退出登录，可能 会有一些 tab 需要隐藏，此时顺序也可能会错乱。

而我做的新需求，恰巧就是会有 tab 变化的(不然我也不会发现这个问题)。当我添加一个 tab 后全部错乱了时，我只能坐在电脑前凌乱···

如何修改

修改的办法也很简单，就是不存储 position 的映射关系，存储任意一个恒定不变的关系，比如 tab 的 id 或者 类型 或者 其他自定义的东西，反正就是一个 key。

而 position 怎么获取呢？ 就通过遍历 tab 或者 adapter(看你怎么封装了，都放进 adapter 好一些吧)，比较 key 是否相等，返回 position。

这样，无论 tab 如何变化，adapter 中的数据也一定会跟着变化，拿着 key 永远找不错。如果找不到这个 key，那只能说明数据变化后，这个 tab 被删除了，那你就得自己看业务需求让你展示哪个页面，再写一段兜底逻辑。

吸取经验

映射关系 key 的选取要考虑全面，任何情况下都不能变动，否则这个映射关系肯定存在问题。

反例2-tab 颜色渐变

还是拿知乎 app 来举例，假设顶部 tab 中有两个 tab 的文字颜色跟其他 tab 不一致，并且滑动的时候还想渐变到对应的颜色，怎么做呢？

这也恰巧就是我的需求，我要新增一个 tab，tab 文字颜色和其余 tab 不一致。

当我辛辛苦苦找到处理 tab 文字颜色的代码时，我是崩溃的

// 以下为伪代码
colors = new int[] {
    resources.getColor(R.color.color1),
    resources.getColor(R.color.color2),
    resources.getColor(R.color.color3),
    resources.getColor(R.color.color4),
    resources.getColor(R.color.color5)}

 public void onPageScrolled(int position, float progress, int positionOffsetPixels) {
    for (int i = 0; i < tabCount; ++i) {
        Tab tab = getTabAtIndex(i);
        if (position == i) {
            // 一坨
        } else if() {
            // 一坨
        } else if(position == count - 1) {
            // 一坨
        } else {
            // 一坨
        }
    }
 }

一开始我没有细看代码，我本着快速做完需求的想法，想着删删改改来完成需求。结果这段代码看的着实头疼，虽然有很多注释，但是不找原作者感觉很难理解了···而且我改了前面后面错，改了后面前面错。

最后我看到了 position == count - 1 这种判断，我决定不使用这段代码。因为这定是为了一种特殊情况(最后一个 tab)做的逻辑，必然不可能用普适性。

不合理的点

1. color 资源没和 tab 绑定
   tab 的字体颜色，应该是构造函数的参数更好一些，如果参数太多了就用构造器模式呗。目前是五种颜色，假设以后越来越多，这数组还得了嘛···而且 setColor 的时候都是 colors[0]、 colors[1] 这种代码，写着不晕？

2. 特殊逻辑 position == count - 1
   既然是 tab 颜色变化，应该是个基础功能，而不应该有什么特殊的逻辑判断

修改

1. tab 构造函数传入一个 int[] 数组来表示各种颜色。
   一个 tab 目前可以多有3个颜色。为啥？首先是选中状态的颜色，其次是未选中状态时，Fragment 的背景可能是黑色可能是白色，对应有两种未选中状态的颜色。取颜色时，通过 tab.getTabTextColor 取到数组，通过定义常量，比如 SELECTED_COLOR = 0，来表示 color 数组的第一个元素是选中态的颜色。

   于是代码变化为：

    // 原始写法：
    int colors = new int[]{color0, color1}
    setTextColor(colors[0], colors[1], progress)
   
    // 修改后的写法： 
    int[] colors = tab.getTextColors();  
    setTextColor(colors[SELECTED_COLOR], colors[UNSELECTED_COLOR], progress)
   

   我个人认为还是比上面的要好懂很多的。

2. 改掉特殊逻辑
   这块就是纯粹的写逻辑了，我感觉之前的人可能就是没太理清楚逻辑(或者确实需求如此)，导致存在一个特殊判断。

吸取经验

1. 常量最好是定义一个有意义的名字

2. 考虑好一些变量的范围，该放到哪里比较通用、比较好

3. 通用组件越通用越好，特殊逻辑应该搞个接口之类的抛给外面