Abhoryo / APYJsFormValidationBundle

This bundle performs validations of a form in javascript. (i18n compatible and several javascript frameworks supported)
92 stars 23 forks source link

Architecture issue #49

Closed gquemener closed 10 years ago

gquemener commented 10 years ago

Hello there,

Just wanted to drop some thought on your bundle, which is a really good idea.

Problem is that it suffers from a lake of extension point resulting in heavy copy paste whenever trying to modify a tiny bit of its behaviour.

For example, let's say we have many entities on which the validation is not the same (in fact the constraints depends on the values of some of the entities property: if anonymous is true, then firstName must be blank). So the generated javascript to validate it would be different from each entites.

Well, too bad the default behavior is to define once and for all the valdation script: https://github.com/Abhoryo/APYJsFormValidationBundle/blob/master/Generator/FormValidationScriptGenerator.php#L185

This method has more than 300 lines and does everthing...

Instead, it'd be nicer to separate all these responsabilities into different classes (that will also improve testability :+1: which is almost inexistant) and inject them through the DIC into your default generator.

For example, you could have one object to fetch the constraints from the form, one to generate the javascript, one to write it somewhere, etc.

Sadly, I've no time to really dig into your code, and my only solution today is to override your classes (through bundle mecanism), copy/paste your code and remove lines that I don't want.

I might sound really harsh, but It makes me sad to see such great idea with such poor implementation.

recipe commented 10 years ago

Why don't you try to use validation groups for different forms on the same entity? You also may use method described here in order to achieve custom validation on different fields.

I agree that it's time to refactor mentioned class so feel free to contribute. It didn't pretend to a status of the best design when it was started. It just stratched an itch of someone. If you really believe it's been a good idea, it needs in your experienced and sophisticated mind, and valuable time.

Abhoryo commented 10 years ago

I'm totally agree with you two. When I start this bundle, symfony wasn't so complex with validation, so keep every thing inside a simple function wasn't so bad.

I almost finish a project, so I think I can rework a little the bundle before the end of the year.

66Ton99 commented 10 years ago

@gquemener this is new bundle which provide better architecture https://github.com/formapro/JsFormValidatorBundle

gquemener commented 10 years ago

Good news @66Ton99, thanks. I'll try it soon!

Abhoryo commented 10 years ago

@66Ton99 Your bundle is great. I think I can close my bundle and suggest your bundle instead.