more semantic about promises HOT 28 CLOSED

I actually think it was a miss that I didn't mark Promise as final.

Instead of refactoring code to allow subclasses to override the constructor, I'd rather see a release that makes Promise final. I'm not sure what impact that would imply for semver, maybe it could be argued as a breaking change warranting a 2.0 at some point. If so, we can add it to the 2.0 wishlist along with possibly some other helper functions baked into Promise perhaps (like map, filter, etc.).

from promises.

@sagikazarmark
I want to add some feature for my own project
So I make Process extends Promise
but when I use my addon function there is error call no function on Promise
so I think this should be new static not new Promise
@jeskew
as you see that is not in constructor it's just call constructor
when a child class extends parent the breaking change should be care by child not parent

from promises.

Perhaps you could replace new Promise with a function call that does exactly the same (e.g. as below); then extenders could change the class created appropriately if they need to (even if they've changed the signature of the constructor), but the default behaviour remains the same.

// replace $p = new Promise(null, [$this, 'cancel']);
$p = $this->nextPromise(null, [$this, 'cancel']);
// and elsewhere in the class
protected function nextPromise(callable $waitFn = null, callable $cancelFn = null) {
    return new Promise($waitFn, $cancelFn);
}

Obviously not useful if you change Promise to be final though, which I'm generally not a fan of due to the limitations it imposes.

from promises.

Thanks, they were good reading.

I don't think that allowing inheritance really violates the open/closed principal, but I can see that forcing users not to use inheritance is likely to encourage better design, that using final limits your exposed public API, and that Ocramius' rule that they can be made final "if they implement an interface, and no other public methods are defined" limits the downsides to it. Presumably you have to exclude constructors from the "no other public methods" list though (as in the case of Promise here). IIRC my problem with Doctrine and final would not have been a problem had that rule been followed then :D

Anyway, good food for thought, much appreciated.

For @yozman, perhaps something like this would help you do what you need to? Instead of extending Promise you can wrap it an implement PromiseInterface. Add modifications as you need to in order to add the functionality you need, but this allows you to reuse the Promise in a way that is acceptable anywhere a PromiseInterface is accepted and still lets you return your own wrapper from .then.

<?php

use GuzzleHttp\Promise\Promise;
use GuzzleHttp\Promise\PromiseInterface;

class WrappedPromise implements PromiseInterface {
    protected $promise;

    public function __construct(Promise $promise)
    {
        $this->promise = $promise;
    }

    public function then(callable $onFulfilled = null, callable $onRejected = null)
    {
        return new static($this->promise->then($onFulfilled, $onRejected));
    }

    public function otherwise(callable $onRejected)
    {
        return $this->promise->otherwise($onRejected);
    }

    public function getState()
    {
        return $this->promise->getState();
    }

    public function resolve($value)
    {
        return $this->promise->resolve($value);
    }

    public function reject($reason)
    {
        return $this->promise->reject($value);
    }

    public function cancel()
    {
        return $this->promise->cancel();
    }

    public function wait($unwrap = true)
    {
        return $this->promise->wait($unwrap);
    }
}

// Example usage
$noop = function () {};
$promise = (new WrappedPromise(new Promise($noop, $noop)))->then($noop);
assert(
    $promise instanceof WrappedPromise,
    '.then() returns a WrappedPromise'
);

Edit: or perhaps that should be

final class WrappedPromise implements PromiseInterface {
    protecteed $promise;

from promises.

@elyobo
thanks for your answer.
maybe your solution is the best way right now.

could you please help me for apply
https://github.com/sinoci/sinoci/blob/master/app/Services/Process.php

from promises.

Can you please explain? I don't see any added value apart from making hacks which rely on inheritance possible.

from promises.

It's unsafe to assume a child class has a constructor compatible with that of its parent. If anyone is using subclasses of promise in their code, this could easily be a breaking change.

from promises.

Putting this onto my close in a few days list.

from promises.

What you could do is overriding then and otherwise to always wrap the returned Promise with yours. The other option is that you always store the new promise in an internal variable and just return $this. For this it's probably better to implement PromiseInterface instead of extending promise, but I am not sure this latter solution would work with your logic.

Ultimately: you can just copy the Promise class and make your changes. The Fulfilled/Rejected promises are created that way. IMO that's the cleanest solution.

when a child class extends parent the breaking change should be care by child not parent

Based on what do you say that? The child's constructor signature does not have to be compatible with the parent's, so relying on it is not safe. There is no such thing as the "child needs to take care of it", because there is no such restriction.

from promises.

from promises.

@sagikazarmark
about extends parent.
I mean if there will be a new breaking change,
when child extends parent,
should let child fix it, not parent.
so there should be new static I think
thanks for your answer

@mtdowling
make new useful function
like next and fail
now then is only for callable
but callable is not clear for read and no arguments.
I want to use like $promise->next('Modal@action', $arg1, $arg2)

from promises.

I'm sorry, I don't understand your arguments (on the technical level).

from promises.

I don't want to override the constructor,
I just want to add some useful function to Promise
for my own logic.
but because of then is return only Promise,
not my subclass,
so there can't find addon function on Promise

from promises.

@elyobo
I can't override then method,
because of some attribute for Promise is private
that means I should overrider them too.

from promises.

from promises.

@elyobo
did u read the source file?
there is too many private attributes to override.
so your suggestion isn't a good idea

from promises.

from promises.

@mtdowling Opened an issue for making promise final

@elyobo what limitations? That you can't misuse inheritance? You have an interface, so you can do whatever you want (except inheriting from Promise)

from promises.

@sagikazarmark I see that you have some strong opinions about what the proper way to use inheritance is, and that any extension of Promise is not proper.

Without commenting specifically on this library, I think that marking things final and private is to assume that you have imagined all possible uses for extension and that they are all invalid. I don't think I'm clever enough to do that, so my preference in general is to leave things more open and if people shoot themselves in the foot because it, that's on their heads :D There is useful code in Promise that people may want to reuse and the privates make it difficult and the final more or less impossible without cut and paste.

from promises.

about what the proper way to use

rather about the improper way which is usually the case

you have imagined all possible uses for extension and that they are all invalid

Not quite. Promise is still open for extension via the interface and composition patterns. It's closed for inheritance though, which people tend to misuse as "the only way" for extension.

from promises.

from promises.

I am sorry, I don't see what "valid use cases" mean here. What are the cases which can ONLY be solved via inheritance?

from promises.

from promises.

extension

The thing is that "extension" is a bit misleading. By extension you mean extending the class. For that I say inheritance. Extension in a wider term however means functional extension of a module/software component. This is what O is from SOLID: Open/Closed principle.

Open for extension in our case means that we have an abstraction which we can use to extend the existing software components. For example write a decorator which adds your logic to the existing one.

People regularly (mis)use inheritance for extension, because it's easy to do. But in many cases it is just wrong, because that way you might actually modify the behaviour from the inside instead of extending it. Classes not meant to be extended (inherited) should not be extended (inherited). That's where the final keyword helps.

If you want to read more about this topic, there are awesome posts by awesome people:

http://verraes.net/2014/05/final-classes-in-php/
http://ocramius.github.io/blog/when-to-declare-classes-final/
https://codeblog.jonskeet.uk/2013/03/15/the-open-closed-principle-in-review/
https://8thlight.com/blog/uncle-bob/2014/05/12/TheOpenClosedPrinciple.html

from promises.

from promises.

from promises.

@yozman see https://github.com/sinoci/sinoci/pull/5

I don't know enough about your application (and there are no tests!) so I have no idea whether this actually works for you, but it demonstrates the idea at least.

from promises.

@elyobo
thanks for your help.
then is work, otherwise doesn't
but I think I got that point. : )

from promises.