Как сделать код ревью?



@JastaFly

Доброго всем времени суток! Попросили сделать код ревью:

<?php

if (!defined("B_PROLOG_INCLUDED") || B_PROLOG_INCLUDED!==true) {
    die();
}

use BitrixMainDataCache;

$cache = Cache::createInstance();
/*ttl 604800 is week*/
if(!$cache->initCache(604800, 'library_classifiers_result_'.$arParams['SEARCH'] ?? 'nosearch', '/iblock/library_classifiers/')) {
    $cache->startDataCache();
    $tmpArResult = [];
    foreach($arResult as $section => $items) {
        foreach($items as $item) {
            if(isset($arParams['SEARCH'])) {
                $continueFlag = true;
                foreach($item['PROPERTIES'] as $propAr) {
                    if($propAr['CODE'] === 'depth' || $propAr['CODE'] === 'PARENT_CODE') {
                        continue;
                    }
                    if(strpos(strtolower($propAr['~VALUE']), strtolower($arParams['SEARCH'])) !== false) {
                        $continueFlag = false;
                        break;
                    }
                }
                if($continueFlag) {
                    continue;
                }
                if(isset($item['PROPERTIES']['PARENT_CODE']['~VALUE'])) {
                    $tmpParents = [];
                    if(!isset($tmpArResult[$section][$item['PROPERTIES']['PARENT_CODE']['~VALUE']])) {
                        $anParent = $item['PROPERTIES']['PARENT_CODE']['~VALUE'];
                        while (!isset($tmpArResult[$section][$anParent]['SECTION_ID']) && isset($anParent)) {
                            $parent = [];
                            foreach($items as $itemTmp) {
                                if($itemTmp['FIELDS']['CODE'] === $anParent) {
                                    $parent = $itemTmp;
                                    break;
                                }
                            }
                            handleElement($parent, $tmpParents, $section, $arParams['SEARCH']);
                            $anParent = $parent['PROPERTIES']['PARENT_CODE']['~VALUE'] ?? null;
                        }
                        $tmpParents[$section] = array_reverse($tmpParents[$section], true);
                        $tmpArResult = array_merge_recursive($tmpArResult, $tmpParents);
                    }
                }
            }
            handleElement($item, $tmpArResult, $section);
        }
    }
    $cache->endDataCache($tmpArResult);
}
else {
    $tmpArResult = $cache->getVars();
}
$arResult = $tmpArResult;

function handleElement($item, &$tmpArResult, $section, $search = false) {
    foreach($item['PROPERTIES'] as $propAr) {
        if($propAr['CODE'] === 'depth') {
            if($propAr['~VALUE'] > 1) {
                $tmpArResult[$section][$item['FIELDS']['CODE']]['DEPTH'] = $propAr['~VALUE'];
            }
            continue;
        }
        if($propAr['CODE'] === 'PARENT_CODE') {
            $tmpArResult[$section][$item['FIELDS']['CODE']]['PARENT_CODE'] = $propAr['~VALUE'];
            $tmpArResult[$section][$propAr['~VALUE']]['CHILDES'] = true;
            continue;
        }
        $tmpArResult[$section][$item['FIELDS']['CODE']]['VALUES'][] = $propAr['~VALUE'];
        $tmpArResult[$section][$item['FIELDS']['CODE']]['NAMES'][] = $propAr['NAME'];
        if($search !== false && $tmpArResult[$section][$item['FIELDS']['CODE']]['INACTIVE'] !== true && strpos(strtolower($propAr['~VALUE']), strtolower($search)) === false) {
            $tmpArResult[$section][$item['FIELDS']['CODE']]['INACTIVE'] = true;
        }
    }
    $tmpArResult[$section][$item['FIELDS']['CODE']]['SECTION_ID'] = $item['SECTION_ID'];
}

Это модификатор компонента Битрикса. Опыта в этом деле у меня не особо много, поэтому подскажите на что стоит обращать внимание? Пока мне тут не нравится куча ифов. Почему не использовать else if? Вообще что лучше с точки зрения производительности куча if или else if или switch case?


Решения вопроса 0


Ответы на вопрос 3



@zorca

А зачем Вас заставили делать код ревью, если Вы даже не знаете, как должно быть? Почитайте код Symfony, попробуйте понять почему так написано… и увольтесь с работы, где заставляют ревьюить код Битрикса. Ведь тут всё гавно, начиная отсутствия единой точки входа в приложение и заканчивая псевдо-ООП в стиле вызова статических методов.



@SerafimArts

Ну… как бы так сказать. С ревью будет сложно, так как тут плохо абсолютно всё. Ну т.е. вообще каждая строчка, кроме “use BitrixMainDataCache;”. Её переписывать, думаю, не надо. Остальное под нож.

Фиг знает как учиться ревьювить то, что надо просто удалить и написать заново, честно.



@usdglander

Если index0h не против, то отвечу ссылкой Попросили проверить код, на что смотреть нужно?

upd: или даже https://github.com/index0h/php-conventions

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *