-
Notifications
You must be signed in to change notification settings - Fork 145
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
Too many issues for me, but best solution, so i will write it here #100
Comments
@6562680 are you interested in working with me on a fork? I just updated the lib to php8 and made it at least pass phpstan level 5. |
i've no practice with fork. almost all the validators even in java is not good for me, but in architectural style have known requirements to create awesome validation library. i mean - best things from several validators that could be extended in future without rewriting whole library. |
I think this library here is a dead project, I don't expect to get an answer here, so I forked it and started updating it. https://github.com/floriankraemer/validation/tree/refactor I just added phpstan and level 5 for now. Level 6 is already showing over 120 errors. I fixed the most obvious stuff phpstorm and phpstan reported, like weak type checks, and also added Github actions, because Travis is a PITA.
My plan is to refactor as much as possible without BC breaking changes and then change whatever is needed to make it better and release a new major version. I introduced interfaces for some classes as well that haven't had an interface before. I already increased the PHP version constraint to ^8.0 because I personally am not willing to maintain code for old PHP versions when I'm doing it for free in my personal time.
If you can do a PR I'll merge it in my fork. I plan to merge a few of the existing PRs from this repository as well. I'll plan to address your other concerns as well because they are valid concerns and good ideas. I'm going to add phpbench to monitor CPU and memory ususage, especially to check your concern in point 6 of your list. |
There's no
nullable
validator like in laravel. It allows to skip validations if user doesnt send the valueYou hardcoding "new Rule", "new Validator" statements and dont use dependency injector (as i see you prefer
extends
instead of decorator injection), so if i want to change default settings of all validators - i should copy and extend all the rules tree. Now i have 30 your validators, and 52 mine. I do nice work to prepare "complete solution" for my projectYou forget method "setImplicit" in Rule, so if i want to change default implicit statement - i need to extend Rule class, but it is hardcoded in Validator and Validation, so i need to extend these ones too, and half-code will be copied in my local project folder
You hardcoded typehint return checkers for getMessage and so on statement to return
string
. So now you need to define message for each validator. Allowing null opens for you feature to addnullable
validator that fails, but do not create any errors. But as i said, you hardcode it, and i need to rewrite and copy a lot of classes to solve it.Validators should be able to made nullable by default like
implicit
. If value isnt set - we just stops validation process likeimplicit
is set but without any errors. Its because set nullable near every damn attribute even when project has 20 DB tables - its really boring. 400 properties with its combinations and even should be markednullable
, so Rule can benullable
tooRegistering new validators using
new
statement it is spending non-cheap kilobytes of memory to store the objects. Its normal for frontend where i have access for users 32Gibs of memory, but when i have 128Gibs on my server and 10000 clients - i have only damn 12.8 mb for each one (and yes, its without relational DB that requires 60% of available memory). So i recommend to improve adding registering validators by classname and then create it when validators is needed to use.The problem of
extends
is the PHP interpreter not allows you to controlnew
statement and typehints. You prevent other developers from improving their own features. Would be great to composer will bedependencyInjector
+eventDispatcher
too (because it knows where exactly positioned the files and also he works with class loading. Simple config allow you to make any function as dependency injector, and second config can maybe even allow to decorate any injected class with magic __call() method that raises outer event), but PHP devs prefer crying one-to-one instead of productive talk.So in laravel i saw temporary solution that makes easier extending hardcoded values. See Illuminate\Database\Eloquent\Model file. There you can see methods like "newModelInstance" or "newBaseBuilder" - its about replacing hardcoded values.
All
get{.*}
statements should allow null statement to be returned. Exclusion of this rule - the state-machines: exampleThere you able to define my own getter and next you define "class getter" that includes checking the property is bound. There is NO SOLID of course (yep, no Barbara tits here), juniors/middles can start to CRY right now. But it helps to updating your code later without the pain.
I'd like to talk more and wrote message to your email but you prefer silence. Good for now.
The text was updated successfully, but these errors were encountered: