rakit / validation

PHP Standalone Validation Library
MIT License
834 stars 143 forks source link

Too many issues for me, but best solution, so i will write it here #100

Open 6562680 opened 5 years ago

6562680 commented 5 years ago
  1. There's no nullable validator like in laravel. It allows to skip validations if user doesnt send the value

  2. You 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 project

  3. You 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

  4. 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 add nullable 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.

  5. Validators should be able to made nullable by default like implicit. If value isnt set - we just stops validation process like implicit 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 marked nullable, so Rule can be nullable too

  6. Registering 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 control new statement and typehints. You prevent other developers from improving their own features. Would be great to composer will be dependencyInjector + 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: example

Class A
{
  protected $prop; // null, because requires logic
  public function prop() // : mixed
  {
    // some logic here
    return 'some';
  }
  public function getProp() : string
  {
    if (isset($this->prop)) return $this->prop;

    $prop = $this->prop();

    // some mappers here

    return $this->prop = $prop;
  }
}

There 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.

floriankraemer commented 2 years ago

@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.

6562680 commented 2 years ago

i've no practice with fork. i could just have conversation with you about my experience in validation. i've just complete collecting required things to make library extra useful.

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.

floriankraemer commented 2 years ago

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.

without rewriting whole library.

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.

There's no nullable validator like in laravel. It allows to skip validations if user doesnt send the value

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.