Skip to content

problem with import aliases & export to object#157

Closed
pupkinV wants to merge 1 commit intoonPHP:masterfrom
pupkinV:master
Closed

problem with import aliases & export to object#157
pupkinV wants to merge 1 commit intoonPHP:masterfrom
pupkinV:master

Conversation

@pupkinV
Copy link
Contributor

@pupkinV pupkinV commented Dec 16, 2012

concentration of the code and improvement obviousness

@AlexeyDsov
Copy link
Member

А можно еще раз привести пример какую именно проблему оно решает? Т.е. пример который с этим фиксом работает, а без него не работает?
Что б красиво вставить php код в комментарий, нужно с новой строки написать слитно `` php` и закрывать все опять тремя такими символами.

@pupkinV
Copy link
Contributor Author

pupkinV commented Dec 17, 2012

$form = Form::create()->
    add(Primitive::alias('n', Primitive::integer('number')))->
    import(array('n' => 42));
//не будет работать (но только при нормальном исходе)!
   import(array('n' => 'anystrig'));
//отработает нормально

checkImportResult ошибку випиливает двояко: markGood для содержимого и unset для своей ошибки.
Ну и стайл: условия разные ($result !== null и $result === true).

@AlexeyDsov
Copy link
Member

Тогда опять вопрос - зачем добавлять алиас на примитив которого нет в форме? Оно на это не расчитано.

@pupkinV
Copy link
Contributor Author

pupkinV commented Dec 17, 2012

ну он, какбэ, в форме.
на примере постгре: запрос FROM user AS u вы не сможете начать SELECT user.id

  • я лично против того, чтобы алиас ПРИНУЖДАЛ к такому действию. в какой-то момент может возникнуть ситуёвина когда примитив нужно сместить number > n и добавить number.
  • на двух наших проектах нашлись уники (4 чела) которые, как и я, не сразу въехали.
  • изменение не принципиально, но делает инструмент пластичнее.

@AlexeyDsov
Copy link
Member

По мне так цель PrimitiveAlias должен быть для внешнего пользования на вид полностью независимым примитивом. Только при создании нужно знать что он чей-то алиас. Все остальные проверки $prm instanceof PrimitiveAlias - хаки :) . В реквесте одна проверка убирается - две добавляются, да еще в двух разных классах вместо одного - Form'ы.

Что бы избежать уников можно сделать в Form::add проверку - если PrimitiveAlias, то нужно проверить есть ли уже его алиас в форме.

Если вдруг кто-то будет переименовывать примитивы добавленные, то я не уверен что форма к этому отнесется хорошо.

Необходимость в текущем хаке связанном с markGood, думаю, пропадет, если-когда @soloweb накатит часть реквеста (о котором напомнил сегодня), отвечающую за вынос ошибок из формы в примитивы.

@pupkinV
Copy link
Contributor Author

pupkinV commented Dec 17, 2012

понял. у нас разные представления. тему можно закрыть ибо для меня не принципиально. в своей версии, для совместимости, выпилю.
забегая на перед: есть идея добавить в мету formAlias и default который потерялся при obj2form

@pupkinV pupkinV closed this Dec 17, 2012
@pupkinV pupkinV mentioned this pull request Dec 19, 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.

2 participants