Вынести цикл перебора позиций строки из кода конкретных матчеров в базовый класс
Issue #55
closed
Originally reported on Google Code with ID 55 ``` Цикл перебора позиций строки (и возможное его отсутствие при якорении) является общим для всех матчеров (кроме preg_php_matcher разумеется). С целью консолидации общего кода его следует вынести в абстрактный класс матчера.
Функция match_inner должна будет осуществлять цикл и вызывать match_from($str, $startpos) для осуществления конкретного матчинга матчерами.
Пока прошу всех участников опубликовать здесь код своих функций match_inner : цикл перебора, а также все что выполняется до и после него.При этом заменить на комментарий с многоточием то, что войдет в match_from, если оно присутствует в match_inner. Так нам будет легче решить, как обобщить этот код. ```
Reported by `oasychev` on 2011-11-20 16:59:22
Comments (14)
-
-
reporter
``` Ваш вопрос про match_inner показывает, что вы невнимательно прочитали мое исходное сообщение - или не подумали над ним. Там упоминалось что есть еще preg_php_mathcer который выполняет матчинг непосредственно через функцию preg_match языка php (для случаев, когда нужны не поддерживаемые нами операции или 100% оттестированный код, хотя подсказок при этом нет). Ему этот цикл не нужен - он уже внутри preg_match. А вот match_inner - нужна.
Поэтому наши матчеры будут перегружать match_from, а матчеры которые пользуются какими-то внешними инструментами матчинга - match_inner, они нужны обе.
P.S. Передайте привет Алексею по поводу этого issue, он я так понимаю не очень читает почту с Google Code. Можно и Дмитрию тоже...
P.P.S. is_new_result_more_suitable тоже должна будет войти в общую часть, так что давайте ее сюда... ```
Reported by `oasychev` on 2011-11-20 17:59:03
-
``` public function is_new_result_more_suitable(&$oldres, &$newres) { if (($oldres->state != $this->automaton->endstate && $newres->matchcnt >= $oldres->matchcnt)
new match is longer ($newres->state == $this->automaton->endstate && $oldres->state != $this->automaton->endstate)
new match is full ($newres->state == $this->automaton->endstate && $oldres->state == $this->automaton->endstate && $newres->matchcnt >= $oldres->matchcnt)) { new match is full and longer return true; } else { return false; } }
Здесь надо будет поменять сложные проверки state и endstate на isfullmatch... ```
Reported by `vostreltsov` on 2011-11-20 18:03:46
-
reporter
``` Цикл перебора вынесен в рамках рефакторинга 2.2, изменения вытолкнуты в соответствующий клон.
Прошу авторов матчеров смержить и перетестить свои матчеры, возможны небольшие изменения и сбои - полный запуск кросс-тестов был проблематичен из-за до сих пор не восстановленного после issue 80 ДКА-матчера.
В ходе анализа были выделены замечания к реализациям функций матчинга, которые желательно устранить. ```
Reported by `oasychev` on 2012-01-06 11:25:56
-
reporter
``` Валерий
Функция match_from_pos содержит странную смесь работы со свойствами класса и нет: с одной стороны она вроде как накапливает результаты совпадения в локальной переменной $results, а с другой стороны при этом - непонятно зачем - использует $this->matchresults
Предлагаю: 1) отказаться от использования $this->matchresults внутри, это свойство для результата (причем сохраняет его туда match), а не временных данных - задача match_from_pos - вернуть результат 2) отбирать лучшие результаты по мере их появления, не накапливая полного массива - результатов может быть много, экономим память 3) хранить результаты как объект qtype_preg_matching_results или иметь в них функцию, возвращающую такой объект по результату 4) использовать qtype_preg_matching_results::worse_than для отбора как единую инстанцию, вместо большей части is_new_result_more_suitable - см. также как это у меня реализовано 5) использовать инвалидизированный объект результата вместо null - см. мою реализацию match_inner 6) избегать условий типа ($result->length[0] > 0 || $result->full) внутри match_from_pos (и т.д.), используя вместо этого функцию qtype_preg_matching_results::is_match т.к. эти условия могут меняться и лучше иметь их централизовано 7) отбор результатов вполне может использовать также qtype_preg_matching_results::best для прекращения дальнейшего поиска совпадения если найдено уже полное...
После такого "причесывания" код станет немного понятнее и значительно управляемее, а также будет экономнее использовать память.
```
Reported by `oasychev` on 2012-01-06 11:28:02 - Status changed: `Fixed`
-
reporter
``` Дмитрий
1) Вы неправильно поняли $endanchor в свое время, пора убрать этот параметр из compare т.к. точное совпадение работает через ассерты, а якорение - вопрос производительности 2) я бы перешел на использование объектов qtype_preg_matching_results внутри compare вместо ваших собственных stdClass-овых объектов ```
Reported by `oasychev` on 2012-01-06 11:28:38
-
``` Согласен со всем упомянутым, код пора причесывать, НО - придется поменять узел обратной ссылки. Он ссылается на матчер и берет оттуда значения подмасок, а они разные в каждом состоянии автомата. Поэтому я их обновляю каждый раз, в коде видно. Можно передавать в функцию матчинга обратной ссылки два массива - начальные индексы и длины. Криво, конечно, но такова особенность обратных ссылок. Может, можно как-то лучше сделать? ```
Reported by `vostreltsov` on 2012-01-06 12:07:15
-
reporter
``` Я бы передал в обратную ссылку дополнительные параметр, что-нибудь вроде $matchdata, который она в свою очередь может передать матчеру запрашивая значение подмаски. Параметру обеспечить значение по умолчанию null.
Этим параметром можно сказать о каком состоянии идет речь и какая подмаска нужна.
Возможно этот параметр придется добавить всем листьям из-за этого, но опять таки со значением по умолчанию это не должно вызывать проблем для матчеров не поддерживающих подмаски. ```
Reported by `oasychev` on 2012-01-06 12:29:39
-
``` Проблема не в этом. Ну передадим мы матчеру запрос на подмаску, но откуда он будет знать её значение, если внутри match_from_pos мы не меняем matchresults? Запрашивать нужно не у матчера, а у автомата. Поэтому я бы напрямую из функции передавал индексы и длины. ```
Reported by `vostreltsov` on 2012-01-06 17:06:38
-
reporter
``` Матчер знает автомат и может запросить у него все что надо. И любой матчер может заводить себе новые поля, помимо полей родительского класса. Зачем использовать именно matchresults?! Использование одной и той же переменной (поля) для разных целей никогда не было хорошей практикой.
Я бы не стал так легко соглашаться на передачу именно индексов и длин. Достаточно ли их будет для работы ЛЮБОГО узла (листа)? Не понадобится ли каким-нибудь условным подмаскам или при реализации рекурсии (в backtracking) другие данные? Таким путем мы никогда не стабилизируем интерфейс этой функции... ```
Reported by `oasychev` on 2012-01-09 16:46:27
-
reporter
``` В общем стоило бы завести отдельную функцию матчера (публичную), возвращающую нужные данные для обратной ссылки отдельно от matchresult.
И в любом случае нет никакой нужды каждый раз создавать новый объект qtype_preg_matching_results, достаточно сделать это один раз, а потом просто инвалидизировать или перезаполнять. ```
Reported by `oasychev` on 2012-01-10 07:53:51
-
``` У нас есть функция preg_matcher::is_subpattern_captured($subpattern). Я предлагаю её немного подправить: возвращать false если подмаска не захвачена, иначе массив с ключами 'index_first' и 'length' для запрошенной подмаски. Детали реализации уже скрыть в наследниках, а в абстрактном матчере всегда возвращать false. ```
Reported by `vostreltsov` on 2012-01-22 15:00:55
-
reporter
``` Функции типа is_xxx возвращают да-нет.
Обратите внимание, что все функции с результатами матчинга сейчас переносятся в qtype_preg_matching_results, в матчере они устаревшие и будут удалены.
Но мне в принципе не очень нравится что одна и та же функция используется и в процессе матчинга, и для возвращения результатов. Это разные по сути вещи... ```
Reported by `oasychev` on 2012-01-23 12:40:31
-
reporter
Reported by `oasychev` on 2012-03-09 19:38:36 - Status changed: `Done`
- Log in to comment
``` Непонятно тогда, зачем вообще нужна match_inner()? Если она делает цикл и больше ничего, то можно же это и прямо в match() сделать? В любом случае, придется стандартизовать возвращаемое значение из match_from(). Я бы просто вызывал из match() функцию match_from() в цикле.
function match_inner($str) { if ($str === '') { return; } $result = new processing_state(...); $startpos = 0; $textlib = textlib_get_instance(); $len = $textlib->strlen($str); match from all indexes for ($j = 0; $j < $len && !$result->isfullmatch; $j++) { $tmp = $this->match_from_pos($str, $j); if ($this->is_new_result_more_suitable(&$result, &$tmp)) { $result = $tmp; $startpos = $j; } } save the result $this->is_match = ($result->matchcnt > 0); $this->full = $result->isfullmatch; foreach ($result->subpatt_index_last as $key=>$subpatt) { if ($subpatt != -2) { $this->index_first[$key] = $result->subpatt_index_first[$key]; $this->index_last[$key] = $result->subpatt_index_last[$key]; } }
generate a character if (!$result->isfullmatch) { $path = $this->determine_characters_left($str, $startpos, $result); if ($path !== null) { $this->next = $path->next; $this->left = $path->matchcnt - $result->matchcnt; } else { $this->next = ''; $this->left = 10000000; the end state is unreachable } } else { $this->next = ''; $this->left = 0; } } ```
Reported by `vostreltsov` on 2011-11-20 17:31:33