Skip to content

PrimitiveRule, изменение внутренностей Form::addRule, checkRules и т.д. #169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

AlexeyDsov
Copy link
Member

PrimitiveRule, вырезанный из #101. Оно мне понравилось и хочется заюзать.

Лирическое отступление. Решил вырезать его оттуда т. к. все вместе выглядит большим и пугающим, а добровольцы желающие его дописать пока еще не разобрались как дружить с тестами, и не сильно загорелись желанием разделять код на части.

Идея следующая

  • Убираем из формы отдельный массив rule
  • Делаем класс PrimitiveRule который болтается взамен в общем списке примитивов у формы, делаем что б валидировал Form'у через метод import
  • не даём ему импортиться при вызовах Form::import*
  • но импортим его при вызове Form::checkRules
  • API расширяется, BC сохраняется
  • Методы Form::addRule($name, $expression) остаются т. к. порой удобней использовать их
  • Из изначальной задумки убрано свойство PrimitiveRule->form и соответственно метод PrimitiveRule::setForm, взамен них примитив ожидает что при импорте вторым аргументом будет передана форма.
  • на всё про всё ушло:

@stev
Copy link
Contributor

stev commented Dec 26, 2012

Убираем из формы отдельный массив rule

rule должны отрабатыватся после того как проимпортятся примитивы

в текущем варианте нужно будет дважны проходить по списку проверяя не Rule ? для импорта
а потом еще разок для рулей

  • оверхед как не как, малый но есть

я бы оставил масив rule, чтоб не делать лишних пробежек по примитивам.

@pupkinV
Copy link
Contributor

pupkinV commented Dec 27, 2012

а помнится там был PrimitiveRuleTest. В нем, что ничего путного не нашлось?

@AlexeyDsov
Copy link
Member Author

rule должны отрабатыватся после того как проимпортятся примитивы

С чего вдруг? Сделали ВСЕ импорты - вызвали проверку правил через checkRules прошли по всем Rule'ам.

а помнится там был PrimitiveRuleTest. В нем, что ничего путного не нашлось?

А что там есть такого чего нет в текущем тесте?

а кто то c конфигами и у них настройка таймзоны в объявлении класса

так в той теме и обсуждай

}

return $this;
return $this->primitiveExists($name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pupkinV
Copy link
Contributor

pupkinV commented Dec 27, 2012

@stev есть в этом доля правды: добавляются отдельным методом, запускаются отдельным методом. Так зачем их сливать в один массив с примитивами, чтобы потом использовать хак instansof разделяя их. хм..
Да, checkRules при этом не пострадает если ошибки писать в один массив

@anisimovt
Copy link
Contributor

PrimitiveRule выглядит странно, правило ведь совсем не примитив. Правило может заниматься всей формой хоть всеми остальными примитивами, тогда возникает вопрос почему у нас есть примитив, который не умеет импорт и проверяет/меняет другие примитивы, да еще и вызывается в другом, специальном методе. Может быть всё же подумать как сделать правила красивее, хотя они меня в текущем виде не только устраивают, но и очень нравятся, у меня куча всего на них завязано.

@AlexeyDsov
Copy link
Member Author

@anisimovt 1) BC не ломается. Form::addRule можно использовать всё так же дальше 2) Правила в таком виде как есть не могу использовать в EntityProto т.к. там задается только список примитивов. 4) Я не в восторге от работы с двумя массивами ошибок errors и violated в методах getErrors, hasError, markCustom и т.д.

@pupkinV
Copy link
Contributor

pupkinV commented Dec 27, 2012

соглашусь с @anisimovt import не просто притянут за уши, но единственный параметр который был теперь nill, а добавился $form который null при нормальном использовании =)
@AlexeyDsov все равно checkRules вызывает метод только у Rule. Может не будем в один метод склеивать 2. Это уже совсем не наследование.

@anisimovt
Copy link
Contributor

Может это тогда проблема EntityProto, а не формы? Я не спорю что не ломается BC, но и мирится с тем что правила, которые что-то делают с примитивами, тоже становятся примитивами не хочется. И то что они не импортируютя и проверяются в другом месте тоже не красиво. Я не отрицаю саму идею, но может её можно сделать по другому? Честно сказать неудобства от работы с 2 массивами ошибок не испытвал, они всё равно объединяются в getErrors(). Ну и ожидание формы в методе импорта примитива совсем меня с толку сбивает

@AlexeyDsov
Copy link
Member Author

@anisimovt таки убедил в переписке что оно для моих целей не стоит. в новом году сделаю некоторые изменения связанные с EntityProto

@AlexeyDsov AlexeyDsov closed this Dec 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants