Remove Model::{asModel, saveAs, newInstance} methods

[mvorisek]

The removed methods were more or less confusing...

Model::asModel replacement:

see removed code, adjust for your usecase (that is why we removed this method, sometimes ID or default values should be copied, sometimes not)

Model::saveAs(string $class, array $options = []) replacement:

$model->asModel($class, $options)->save();

Model::newInstance(string $class = null, array $options = []) replacement:

new $class(null, $options);

This works even with $class beiing an object - https://3v4l.org/EfOiW

This PR also improve/simplify Model::asModel() method.

No change in atk4/ui repo needed.

[DarkSide666]

You can do simply $m->set($this->data); here without loop.
But I'm not sure why you removed condition lines above. There was some logic - to not copy null and default values and also of course not copy id.

[mvorisek]

• ignoring null - why? (I think too much crazy logic)
• ignoring v = default - saves only a call, nothin else :) (should be removed)
• ignoring ID - why (seems wrong to me, different model usually still targets the same data/table)

We would then need loadAs... This is too much methods for nothing.

[DarkSide666]

This is the essence of newInstance(). At least for me.
It creates new model object and already links it with same persistence of old object.

[mvorisek]

I am aware, I can not be fixed in the constructor as construtor is not aware of the previous instance/persistence.

The solution is to call new $class($model->persistence), eg. pass the persistence manually when needed.

Safety is handled, all DB operations throws if persistence is not set.

[DarkSide666]

And this was quite a nice feature we used in one project :)

[mvorisek]

See code below, it tries to return different class from load method.

Notice, that a lot of custom code is needed already, you can adjust the code for the removed method easily. Usually, in the case below, you want to retain ID!

[DarkSide666]

Is George around in Discord sometime? I would love to hear his opinion too on this.
I kind of understand that we can live without these methods (including asModel), but on other hand - why should we? Everything can be done manually in project-specific way, but it's also good if framework does some work for you (at least in one specific way and if you don't like that, then you can overwrite or extend).

So to wrap this up - I will approve this PR, but please try to discuss it also with George before merging. Maybe he can come up with better example of use-case then I could.

[mvorisek]

@georgehristov Please approve as well, asModel is not good as the copy conditions depends on use case and all core functions must behave strictly from the name itself, which is not possible here. newInstance is more or less like an alias for constructor and confusing for the developer. Even for me, and I know a lot of internal things... We cleared this with @DarkSide666, but we want to make sure our decisions are aligned together. Thanks.

[georgehristov]

Ok, approving