Поругайте код, мне будет полезно
Господа, буду рад любым предложениям по улучшению этого написанного мною кода.
Поскольку я дилетант, то наверняка делаю это топорно и не изящно.
Поставил себе цель написать сниппет, который будет выводить на экран значения MIGX TV для всех ресурсов, которые будут дочерними к ресурсу родителю, чей id будет задаваться как параметр сниппета.
Поскольку я дилетант, то наверняка делаю это топорно и не изящно.
Поставил себе цель написать сниппет, который будет выводить на экран значения MIGX TV для всех ресурсов, которые будут дочерними к ресурсу родителю, чей id будет задаваться как параметр сниппета.
<?php
// получаем id родительского каталога как параметр
$p=$parent;
// выборка всех ресурсов, у кого родитель имеет переданный id
// и формируем массив с id ресурсов-детей.
$g=$modx->getCollection('modResource',array('parent'=>$p));
foreach ($g as $g1){
$where[]=$g1->get('id');
}
// проходим по всем элементам нашего массива, делая выборку одного объекта
// значение contentid которого совпадает с id из массива
// поскольку я задался целью получить только MIGX то прописываю id этого объекта
foreach ($where as $nowhere){
$result=$modx->getObject('modTemplateVarResource',array('contentid'=>$nowhere,'tmplvarid'=>1));
// добавил сюда проверку на существование $result поскольку не все ресурсы могут иметь заполненные
// MIGX поля
if ($result){
// $res становится строкой, в которой лежит JSON
$res=$result->get('value');
//$r становится массивом
$r=$modx->fromJSON($res);
// перебираем все елементы массива, чтобы добраться до содержимого.
// У меня поле имеет название text
// в результате получаем вывод на экран перечня значений всех MIGX для всех ресурсов
// дочерних к тому, чей id мы передали в сниппет
foreach ($r as $r1){ echo $r1['text']."
";}
} //end if
} // end foreach
Комментарии: 8
Авторизуйтесь или зарегистрируйтесь, чтобы оставлять комментарии.
Не надо так делать.
И вообще, если у вас есть переменная $result, то должно быть return $result, что очень логично)
2. В сниппетах не нужно делать echo, только return.
Насчет названий переменных, Вы правы.
Насчет echo — тоже, хотя я ставил целью именно вывести на экран.
Кстати, раз уж мы коснулись echo и return — Вы замечали, что сниппет modx печатает на экран то, что вызывается как return?
В нем return ведет себя не как в PHP. Поэтому применение return в моем конкретном случае привело бы к показу первого результата.
Если задаваться целью сделать вывод в чанк, то наверное, правильно было бы в цикле формировать строковую переменную с вызовом getChunk() а уже в конце делать return этой переменной.
лучше создать переменную $output и выводить потом ее
Второ момент, зачем тут такие странные манипуляции?
Лучше использовать getIterator, на большом количестве ресурсов памянть не закончится, это раз. Два раза вы делаете проход по массиву, абсолютно бесполезно. Вы же и так перебираете ресурсы, почему не сделать вот так?
Большая вложенность кода в {} ухудшает читабельность кода, поэтому идеальный код — это прямой без ветвлений или стараться их избегать где возможно. Например, вместо проверки, есть ли значение в переменной и выполнения кода внутри if, можно проверить, что если значение пустое, то пропустить остальной код и перейти на следующую итерацию.
Про return из сниппета вам уже сказали, лучше собирать весь вывод в буфер в виде массива или строки и потом уже выводить одним return. Это даст возможность в дальнейшем, когда условия задачи изменятся, еще раз пройтись по массиву или строку и применить дополнительные действия над результатом.
Простой пример, у вас сниппет в зависимости от параметра должен выдавать или строку на экран или json в случае ajax-запроса. В таком случае можно сделать так:
И последнее, несмотря на то, что параметры в снипете уже присутствуют в виде переменных, лучше получать их значения через API MODX. Это в дальнейшем даст возможность использовать Property Sets (предустановленные параметры) и значения по умолчанию.
Ну и в дальнейшем нужно углублять знания SQL и xPDO.
В этом коде делается много запросов к базе данных: сначала получаем все нужные ресурсы (1 запрос), и потом на каждый ресурс делаем дополнительный запрос в базу, чтобы получить значение TV-параметра. При большом количестве ресурсов сниппет будет создавать нехилую нагрузку.
Чтобы такого избежать, используем LEFT JOIN: