记一次代码优化

review团队成员代码时，发现了这一段代码

$sessionMap = SalesMan::withTrashed()->whereIn('id', $saleManIds)->pluck('session_id', 'id')->toArray();
//属于同一用户的数据合并
$retKey = ['chat' => 'chatRet', 'chatUser' => 'chatUserRet', 'attention' => 'attentionRet', 'credit' => 'creditRet'];
foreach ($sessionMap as $saleManId => $sessionId) {   
     if (!isset($sessionToSaleMan[$sessionId])) {
        $sessionToSaleMan[$sessionId] = $saleManId;
        if (isset($callRet[$saleManId])) {
            $callRetTemp[$sessionToSaleMan[$sessionId]] = $callRet[$saleManId];
        }
    } 
    foreach ($retKey as $k => $r) {
        if (isset($r[$saleManId])) {
            if (!isset($k['retTemp'][$sessionToSaleMan[$sessionId]])) {
                $k['retTemp'][$sessionToSaleMan[$sessionId]] = $r[$saleManId];
            } else { 
               $k['retTemp'][$sessionToSaleMan[$sessionId]] += $r[$saleManId]; 
           }        
        }
     }}
$chatRet = isset($chat['retTemp']) ? $chat['retTemp'] : $chatRet;
$chatUserRet = isset($chatUser['retTemp']) ? $chatUser['retTemp'] : $chatUserRet;
$callRet = isset($callRetTemp) ? $callRetTemp : $callRet;
$attentionRet = isset($attention['retTemp']) ? $attention['retTemp'] : $attentionRet;
$creditRet = isset($credit['retTemp']) ? $credit['retTemp'] : $creditRet;
// 格式化
salesmen = array_unique(array_merge(
    array_keys($chatRet),
    array_keys($callRet),
    array_keys($attentionRet),
    array_keys($creditRet),
    array_keys($chatUserRet)
));

首先来看看这部分代码做了什么事：其实是在合并用户的一些业务的统计数据，在我们的系统中，这部分人会有多个身份记录，如果不进行合并处理，一个人的数据会被展示多次，有些业务中分散在不同的身份里，真实数据要累加，有些数据则被重复的展示。

他用动态创建变量的方式，将不同业务合并后的数据分别存入。然后覆盖老数据。

来分析这段代码的问题。

问题一，真的需要中间变量吗

答案是不需要，我们只需要做两件事

• 确定分散的数据要合并到哪个身份上
• 将需要累加的业务数据，累加到这个身份上，其他删除；将不要累加的业务数据，只保留这个身份的数据即可

只需修改老数据，不需要中间变量，更不需要动态创建。

问题二

salesmen = array_unique(array_merge(
    array_keys($chatRet),
    array_keys($callRet),
    array_keys($attentionRet),
    array_keys($creditRet),
    array_keys($chatUserRet)
));

这段代码取最后合并数据后的所有身份id，然而

if (!isset($sessionToSaleMan[$sessionId])) {        
        $sessionToSaleMan[$sessionId] = $saleManId;
        if (isset($callRet[$saleManId])) {
            $callRetTemp[$sessionToSaleMan[$sessionId]] = $callRet[$saleManId];
        }
 } 

问题三，变量命名随意

再加上动态创建，不去往上面翻代码，很难单独理解这一段代码

 foreach ($retKey as $k => $r) {
        if (isset($r[$saleManId])) {
            if (!isset($k['retTemp'][$sessionToSaleMan[$sessionId]])) {
                $k['retTemp'][$sessionToSaleMan[$sessionId]] = $r[$saleManId];
            } else { 
               $k['retTemp'][$sessionToSaleMan[$sessionId]] += $r[$saleManId]; 
           }        
        }
  }}

/** 
 * 同一用户分散在不同身份标签下统计数据统一化
 *
 * @param array $salesManIds, 未去重的salesmanId
 * @param &array $chatRet, 微聊数量统计数据
 * @param &array $chatUserRet, 微聊用户统计数据 
 * @param &array $attentionRet, 关注统计数据
 * @param &array $creditRet, 加分统计数据
 * @param &array $callRet, 来电统计数据
 * @return array, 去重后的salesManId
 */
private static function uniqSalesManAnalysisData($salesManIds, &$chatRet, &$chatUserRet, &$attentionRet, &$creditRet, &$callRet){
    //获取身份标签对应的的session_id
    $sessionMap = SalesMan::withTrashed()->whereIn('id', $salesManIds)->pluck('session_id', 'id')->toArray();
    //变量初始化    
    $uniqSalesManIds = [];
    $retList = [&$chatRet, &$chatUserRet, &$attentionRet, &$creditRet];
    foreach ($sessionMap as $salesManId => $sessionId) {
        if (! isset($uniqSalesManIds[$sessionId])) {
            $uniqSalesManIds[$sessionId] = $salesManId;
        }
        $uniqSalesManId = $uniqSalesManIds[$sessionId];
        //处理retList中包含的统计数据,累加至$uniqSalesManId
        foreach ($retList as &$salesManRet) {
            !isset($salesManRet[$uniqSalesManId]) and $salesManRet[$uniqSalesManId] = 0;
            if ($salesManId == $uniqSalesManId) {
                continue;
            }
            if (!isset($salesManRet[$salesManId])) {
                continue;            
            }
            $salesManRet[$uniqSalesManId] += $salesManRet[$salesManId];
            unset($salesManRet[$salesManId]);
        }
        //处理来电的统计数据，只保留$uniqSalesManId
        if ($salesManId != $uniqSalesManId && isset($callRet[$salesManId])) { 
           unset($callRet[$salesManId]);
        }
    }
    return  array_values($uniqSalesManIds);
}