Вроде я проверил практически все посты в прошлом треде, и ответил на все вопросы, так что если вы что-то спрашивали, зайдите в старый тред >>1380485 (OP) и проверьте. А если я пропустил, напомните о себе тут.

Заодно я бы хотел разобрать одну интересную задачу из прошлого треда. Есть такая известная задача на ООП про скидки, кто мои задачи про ООП решал, думаю, без проблем справится. Вот тут вот задача: >>1395825 а тут решение анона: https://github.com/nokitakaze/test-programming-task-am-items/

Думаю, прошло много времени, так что прокомментирую решение:

> Правило №4 "стоимость выбранного продукта уменьшается на 5%". Что такое "выбранный продукт"? Покупатель произвольно пальцем тыкнул в любой продукт, и у него цена уменьшилась?

Это товар A из правила.

> Если одновременно выбраны А

Вот он и "выбранный".

Вообще, переставлять правила плохая идея. Да, они могут быть не очень логичными, но в задаче требуется сделать именно как описано. Согласен, впрочем, что формулировка плохая.

https://github.com/nokitakaze/test-programming-task-am-items/blob/master/src/Item.php

Тут странный отступ в 4 пробела. Он не нужен.

> protected $_type;

Не надо использовать подчеркивание в именах свойств. Это было в PHP4.

> @var ItemType[]|ItemType[][]|null

> protected $_only_includes = null;

Вот такой зоопарк типов - это плохо. Тебе при работе с полем придется всегда делать проверку на тип данных, ее легко забыть, итд. Это будет плохой, запутанный код.

И вообще, тут ты имхо переусложнил код. Ты пытался сделать так, чтобы у правил было как можно больше общего кода, чтобы достаточно было передать для разных правил разные массивы, но это лишь привело к переусложнению логики. То есть, думаю, что из базового класса Rule можно вообще убрать эти поля exclude/inlcude.

Тебе не надо было в базовом классе делать поля exclude/include. Они не применимы ко всем видам скидок, и значит, им незачем быть в базовом классе. Также, не стоит полагаться на идею "класс-наследник должен переопределить этот массив", так как она никак не задокументирована и не проверяется. Если ты хочешь что-то требовать от класса-наследника, то используй абстрактные методы.

Также, не стоило объединять правила 1 и 4 в один класс. Так как там другая логика. Или если и объединять, то без потери читабельности кода.

Также, посмотри на объем кода в https://github.com/nokitakaze/test-programming-task-am-items/blob/master/src/Rules/TotalCostSelectedItemsRule.php и сравни с тем, как это правило описано в одном предложении в задаче. Этот код стоит хотя бы разбить на методы с понятными названиями, сейчас он вообще не читаем.

Ну например, один из вариантов - это сделать так:

// Сортируем товары по типу
foreach ($types as $type) {
$itemsByType[$typeName] = $this->pickItemsByType($items, $type);
}

// Определяем число групп
$itemsCount = array_map('count', $itemsByType);
$pairsCount = min($itemsCount);

// Проходимся по группам товаров и даем скидку за каждую
for ($i = 0; $i < $pairsCount; $i++) {
$group = $this->selectGroup($itemsByType, $i);
$this->addDiscountForPair($result, $pair);
}

Вот, основная логика кода занимает небольшой объем и гораздо читабельнее. У тебя же код плохо читабелен, а разделение на функции (calculateIteration) не помогает его понять.

Что касается формата ответа, то думаю, стоило сделать объект, который бы хранил: вид скидки, размер скидки, к чему применяется, за какие товары дана. Тогда мы можем в итоге вывести информацию о примененных скидках. Твой же класс RuleCalculationResponse этого не позволяет.

И еще, ты сделал метод Rule#calculate(), который принимает массив товаров и за один проход ищет все скидки. Но можно было еще сделать по-другому: он принимает массив товаров и возвращает первую найденную скидку. Соответственно, мы вызываем его в цикле, пока все скидки этого типа не найдутся. Плюс в том, что это упрощает код правила. Нам не нужно искать все скидки, достаточно найти одну. Для правила 1, например, код будет совсем простой:

function applyDiscount(array $items)
{
$group = [];
foreach ($this->types as $type) {
$item = $this->findItemByType($items, $type);
if (!$item) {
return;
}
$group[] = $item;
}

// Применить скидку к $group
return new Discount(....);
}

Хотя этот код менее эффективен, но он более читабелен. А если встроить в корзину функцию поиска по типу и оптимизировать ее, то может быть даже более эффективен, чем твой код.

https://github.com/nokitakaze/test-programming-task-am-items/blob/master/src/Rules/RuleGroup.php

В этом классе, наверно, логично было сделать метод вроде applyGroup() или как-то так. А то класс не сильно отличается от просто массива.

https://github.com/nokitakaze/test-programming-task-am-items/blob/master/src/Rules/RuleSet.php

Не очень понятно, зачем этот класс.

https://github.com/nokitakaze/test-programming-task-am-items/blob/master/test/CalculatorTest.php

Тест сложный. Его тяжело прочесть, и понять, а правильный он или нет? В случае падения - непонятно, что именно не так. Обычно тесты пишут для проверки отдельных требований. В твоем случае, можно сделать такие тесты, начиная с тестирования отдельных скидок:

- тест, что скидка на группу товаров применяется, когда она есть
- что скидка применяется несколько раз, если групп несколько
- что она не применяется, если товаров нет
- тесты на другие виды скидок ....
- тест на то, что 2 разных скидки могут применяться к одной корзине
- тест на то, что товар, участвующий в одной скидке, не используется в другой

То есть тестируем отдельные простые требования простыми тестами. То, что у тебя - сложновато выглядит, и это скорее приемочные, а не юнит-тесты. Я бы предпочел набор простых тестов.

И не надо в одном тесте тестировать разные объекты (testprimitives). Надо было сделать 2 отдельных теста.

>>24567

Ты занимаешься интересным делом, но, увы, немного неправильно. Я сейчас не могу подробно все разобрать, так как у меня много дел до понедельника, но немного прокомментирую.

Ты разбираешь исходный код регулярками тут: https://github.com/ta-tikoma/phpins/blob/master/app/Validators/IDefinedThisVariable.php

Это неправильно. Например, твой разборщик кода "увидит" функцию, даже если она закомментирована или находится внутри строкового литерала:

// function x() {}
$s = 'function x() {}';

Также, твоя регулярка некорректно определяет границы заголовка функций, например:

function[^\(]+\(([^)]+)\)

Тут не учтено, что определение функции может содержать вложенные скобки:

function x($a = array())

Также, у тебя нет поддержки синтаксиса декомпозиции массивов:

[$a, $b] = [1, 2];

И не учитывается то, что некоторые функции принимают аргументы по ссылке и могут таким образом "создавать" переменные:

preg_match("/x/", "x", $m);

Правильный способ разбирать код - это лексический и синтаксический анализ. Лексический анализ - это разбор исходного кода на список лексем (токенов), где каждый токен является неделимым кусочком кода. Например:

if ($a == f($b)) {
echo $x; // yes
}

превратится в: 'if', '(', '$a', '==', 'f', '(', '$b', ')', ')', '{', 'echo', '$x', ';', '}'

На этом этапе удаляются лишние пробелы и комментарии.

Для проведения лексического анализа в PHP есть готовые функции: https://php.net/manual/ru/book.tokenizer.php

Затем производится синтаксический анализ. Он берет список лексем и используя набор правил, воссоздает синтаксическое дерево (AST) исходного кода. Набор правил называется грамматикой, и он описывает все возможные конструкции, которые могут встретиться в коде. Грамматика описывает, как из неделимых кусочков (их еще называют терминалами) собираются сложные конструкции, а из них - еще более сложные.

Вообще, в теории можно использовать синтаксический анализ сразу, без этапа лексического анализа, работая с отдельными символами, а не токенами, но обычно все же это делают в два этапа.

Ну например, правило может выглядеть так:

конструкция if - это: токен if, за ним круглая скобка, за ней выражение, за ним круглая скобка, за ней фигурная, за ней команды, за ней закрывающая фигурная.

или, если записать это правило в Расширенной форме Бэкуса — Наура (EBNF):

if_construct = 'if', '(', expression, ')', '{', 'sentences', '}'

Здесь expression и sentences - это отсылки к правилам, описывающим из чего состоит выражение и набор команд. Выражение можно описать как-то так:

# От порядка записей зависит приоритет операторов
expression = function_call | multiplication | sum | comparison | variable-like | literal
function_call = indentifier, '(', [ parameter, { ",", parameter } ], ')'
multipication = expression, ( '*' | '/' ), expression
sum = expression, ( '+' | '-' ), expression
comparison = expression, ( '<', '>', '==', '===', '!=', '!==', '<=', '>=' ), expression
variable-like = array_element | variable
literal = string | number | boolean_constant

Вообще, конечно, правила PHP чуть сложнее, и они позволяют пропускать фигурные скобки, а также допускают операторы elseif/else, так что запись можно усложнить:

if_construct = 'if', '(', expression, ')', body, { 'elseif', '(', expression, ')', body }, [ 'else', body ]
body = '{', many_sentences, '}' | one_sentence

Если тебе не очень понятен синтаксис EBNF, то погугли. Кроме EBNF, есть еще другие формы записи грамматик: ABNF, BNF. Они есть в википедии. Но общая суть в том, что они описывают сборку сложных конструкций языка из более простых. Программа состоит из определений и команд, определения состоят из более мелких частей, и так до неделимых частей - терминалов.

Парсер, используя правила грамматики, разбирает исходный код и воссоздает синтаксическое дерево (дерево - это структура, состоящая из узлов, и у каждого узла могут быть дочерние узлы, и есть один главный корень, с которого начинается дерево). Корнем дерева тут будет "Программа". Внутри нее будут определения классов, функций, и блоки кода. Внутри функции будет список ее аргументов, и тело функции. Тело будет содержать команды, они могут содержать выражения и вложенные блоки кода, и так далее. Дерево будет выглядеть так:

Программа:
- Определение функции
-- Имя: f1
-- Аргументы:
--- Аргумент
---- Имя: $a
---- Тайп-хинт: int
---- Значение по умолчанию:
----- Константа класса: SomeClass::SOME_CONSTANT
-- Тело:
--- Приваивание:
---- Переменная: $a
---- Значение: 1

Парсер, который занимается синтаксическим разбором, писать не надо - есть готовые генераторы парсеров, которым достаточно дать грамматику языка на вход: PHP-Yacc, PHP-peg, Lemon-PHP.

То есть, тебе достаточно описать грамматику PHP и ты сможешь парсить исходный код в дерево AST. После чего ты уже можешь легко обходить его, искать там узлы определенного типа (например: функции) и внутри них анализировать использование переменных. Правда, описать грамматику PHP довольно сложно. Где взять полный набор правил? Они есть в исходном коде PHP:

https://github.com/php/php-src/blob/master/Zend/zend_language_parser.y

Разработчики PHP использовали сишный генератор парсеров Bison и описали грамматику в нужной ему форме. Полистай файл, и ты увидишь там описание грамматики.

Также, разработчики Facebook пытались составить спецификацию PHP, но похоже, забросили ее: https://github.com/php/php-langspec/blob/a610388d637b67294925d3b1dbbc0a54b7d94428/spec/php-spec-draft.md

Но хорошая новость состоит в том, что и это делать не надо. Есть готовый парсер кода PHP: https://github.com/nikic/PHP-Parser

Соответственно, тебе стоит попробовать взять его за основу. Он даст более качественный результат. С его помощью можно даже делать автоматический рефакторинг кода.

И немного о том, как искать неопределенные переменные. Твой анализатор ведь не учитывает вложенность и порядок выполнения кода, например:

if (...) {
$a = 1;
}

$a = f($a);
echo $a;

Тут переменная $a может быть не определена, и это ошибка. А если мы напишем так:

if (...) {
$a = 1;

} else {
return;
}

$a = f($a);
echo $a;

То ошибки уже нет. Для анализа потока выполнения можно использовать преобразование кода в SSA-форму ( https://ru.wikipedia.org/wiki/SSA ). SSA = static single assignment - это когда значение переменной присваивается только в одной команде, а если в коде это делается несколько раз, то мы делаем несколько копий переменной, добавляя суффиксы ($a_1, $a_2 и тд).

Например, код:

$a = 1;
$a ++;
echo $a;

превращается в:

$a_1 = 1;
$a_2 = $a_1 + 1;
echo $a_2;

Если же переменная определяется внутри цикла или if, то мы используем конструкцию "Ф-функция", которая говорит о том, что переменная может получить одно из нескольких значений:

if (...) {
$a = 1;
} else {
$a = 2;
}

echo $a;

превращается в:

$a_1 = 1;
$a_2 = 2;
$a_3 = Ф($a_1, $a_2);
echo $a_3;

Ну и наконец, вот еще пример цикла:

$a = 1;
while ($a < 10) {
$a ++;
}

echo $a;

Он превратится в:

$a_1 = 1;
$a_2 = Ф($a_1, $a_2) + 1;
echo $a_2;

SSA удобна тем, что мы можем идти по цепочке команд обратно, и анализировать, какие значения могут попасть в ту или иную переменную. Так как каждая переменная уникальна, нам не нужно анализировать порядок выполнения кода, достаточно просто найти строку с ее определением. SSA-форма позволяет, например, определить, значения какого типа могут содержаться в переменной, определить какие-то ограничения (то, что оно целое или что оно неотрицательное). Представь, какие тщательные проверки можно было бы делать (обнаруживать, например, что отрицательные числа передаются в функцию, которая принимает только положительные).

Ты бы мог использовать ее для поиска неопределенных переменных.

В общем. советую почитать и погуглить про:

- лексический анализ
- синтаксический анализ
- AST (abstract syntax tree)
- форма представления кода SSA
- формы записи грамматик BNF/ABNF/EBNF
- https://github.com/nikic/PHP-Parser

На первый взгляд это может выглядеть сложно, но работать с кодом в такой форме будет удобнее. Я когда-то пробовал написать простой анализатор кода для поиска ошибок, как раз на основе синтаксического анализа: https://github.com/codedokode/source-analyzer Там правда тоже есть регулярки, но есть и использование AST: https://github.com/codedokode/source-analyzer/blob/master/src/Common/TypeHintMissingChecker.php

Вообще, если бы ты мог написать преобразователь AST в SSA, это могло бы стать полезной отдельной библиотекой для анализа кода.

Если есть вопросы - задавай.