Причесывание кода инструментов авторинга к релизу

Originally reported on Google Code with ID 200
Ниже перечислены замечания по коду инструментов авторинга, они пронумерованы. Про некоторые
из них было говорено еще полгода назад, а воз и ныне там! Давайте приводить в порядок
свой код. Пункты пронумерованы - по каждому исправлению отписывайтесь здесь с указанием
номера пункта, если есть вопросы - тоже.

Всем
1)Методы должны содержать PHPDoc комментарии с объяснением метода, параметров и значения
- на английском языке.
Исключение допускается ТОЛЬКО для перегруженных методов (когда соответствующий комментарий
уже есть в родительском классе).
Каждому отписаться про исправления в своих файлах...

2) Общий стиль заголовков файлов - см. основной код вопроса. Копирайт везде пишем на
меня, так технически удобнее под GPL, есть поле "авторы" для указания авторства...

3) Слить код обработки спец-символов единые методы для всех инструментов с четкими
названиями. Вот этот код можно разместить в статических функциях...
(Валерий - нам с вами подумать, оставить ли функции статическими или в каком классе
они должны быть динамическими? По хорошему эскейпинг для dot например нужен не только
инструментам авторинга - может в poasquestion вынести?)

Языковой файл
4) использование % вместо $a в // Strings for node description - зачем необходимо?
Для номеров, названий подмасок, чисел и подобных неизменяемых вещей точно нет. Да и
для остальных лучше использовать {$a->something} вместо %something - соответственно
уйдут str_replace из кода. $a - официальный способ подстановки одних строк в другие...

Если неясно про $a - спрашивайте...


explain_graph_misc.php
5) Почему функции qtype_preg_author_tool_explain_graph_subgraph статические? Функции
должны быть динамическими, если это возможно - т.е. если нет другого класса, которому
нужно вызывать такую функцию без объекта этого класса... иного оправдания статике нет.
explain_graph_nodes.php
6) сделать отдельные классы для разных видов листьев - вместо switch/if
7) Поддержка опций прямоугольниками (до ближайшей закрывающей скобки), подобно подмаскам
и квантификаторам - отображать опции, если они действуют на прямоугольники
8) process_charset должен иметь комментарии и юнит-тесты, и уж очень он сложный...
обсудить при встрече
9) qtype_preg_authoring_tool_operator_condsubexpr::process_operator содержит очень
странную конструкцию - условие с тремя вариантами типов, внутри switch по ним - логичнее
заменить обычным if ... else if ... else - 
10) switch/if и т.д. с одинаковым кодом, но разными параметрами - типа того, что в
qtype_preg_authoring_tool_operator_assert - лучше заменить на обращение по индексу
в массив... Такой массив можно сделать статическим в классе.
explain_graph_tool.php
11) get_engine_node_name - переделать с использованием node_infix, должна сильно уменьшиться
- сравните с тем, как сделали это два других инструмента
12) Почему функции - статические?! Переменные тем более... (не говоря уже о временной
переменной...) Сделать все динамическим!
13) Сложные функции типа process_asserts должны иметь комментарии по коду!
14) Подчеркивания в именах переменных process_asserts. 
15) основные функции - process_asserts и process_simple должны иметь юнит-тесты
16) Функции cmp_xxx перенести в файл юнит-тестирования. 
17) Юнит-тесты на граф и отдельные функции очень примитивны; мне надо разыскать вашу
программу и методику испытаний с КНПО или сами найдете?!
preg_authoring_tool.php
18) Должен включать в себя, наверное, все промежуточные классы: для инструмента с картинкой
(пока нет - хотя давно говорено!), dot-инструмент (где пока одно TODO...)
preg_description.php
19) Функции должны быть динамическими, если это возможно - т.е. если нет другого класса,
которому нужно вызывать такую функцию без объекта этого класса...
20) Перед многими открывающими фигурными скобками нет пробелов. И после запятых при
объявлении и вызове функций - тоже.
preg_explaining_tree_nodes.php
21) методы типа label() содержат слова - все слова для пользователя должны получаться
через get_string, чтобы быть переводимыми!

Владельцем ставлю пока себя для надзора.
Reported by oasychev on 2013-05-31 19:17:13
Comments (33)
