should we archive this project?

[dbu]

This implementation used to be a demo case for the Httplug interfaces and PSR-18. Meanwhile, guzzle and buzz both implement PSR-18 and symfony also provides an HTTP client that comes with a PSR-18 wrapper.

There are a couple of problems with this client: #55 and #59

Without wanting to devalue the effort put into this, i want to raise the question if we still need to maintain this, or better focus energy on making guzzle | buzz | symfony/http-client as good as possible?

[nicolas-grekas]

Do it: as shown the implementation has severe issues, and fixing them requires someone to work on it. That someone will thank you for not doing it, maintainance included.

[Ocramius]

I'm generally a fan of this library: does what it advertises, without too much clutter, and it is my go-to solution for anything PSR-7/PSR-18.

Long time user, and will gladly replace all of the mentioned libs with a php-http/ lib instead, when there's a chance to do that.

I happily replaced both buzz and guzzle with php-http in the past too :+1:

[sagikazarmark]

The curl client is the most popular client implementation after Guzzle 6. Installation stats haven't decreased, so I'd say it's worth keeping around.

One of those severe issues already has a PR, the other can probably be solved by using guzzle/promise.

[dbu]

thanks for the inputs. then we need some people to work on these things... how important is the async support? as most issues are around that, we could maybe drop the async part in the next major version, to make this thing much simpler and get rid of the promise issues... or would that destroy the usefulness of this client?

[frodeborli]

The first thing that needs to change is that the "MultiRunner" class becomes replaceable (because it is not integrated with any event loop).

Also; please stop using "private" functions and "private" properties. Use "protected" instead - which means that child classes can access the properties.

I'm working on getting this to work with my minimalistic async library (f2/promises) - and private properties is making it much more difficult.

[Ocramius]

Hey @frodeborli: if the MultiRunner is not designed for inheritance, it should be replaced via composition instead, and marked final: maybe send a patch to make an interface for it, so that replacing it is possible.

[frodeborli]

@Ocramius I don't think it should be marked final. It should use protected instead of private - it is a common design mistake that many programmers make. I solved the problem by injecting my own multirunner instance using ReflectionProperty for now. If it turns out to work nicely for my use case, I'll consider sending a patch.

My f2/promises library is intended to solve interoperability issues between other async libraries such as Guzzle, ReactPHP, Amp - and I found out that php-http is an excellent test case for this :)

[sagikazarmark]

It should use protected instead of private - it is a common design mistake that many programmers make

I think it's quite the opposite: it's better to be defensive and make the API surface small, so that internal changes don't affect consumers.

I agree with @Ocramius : composition is the way to go.

[frodeborli]

@sagikazarmark The API surface does not increase if you make methods protected instead of private. People should generally use protected. Private methods make object oriented programming a complete waste of time.

Composition should be used also; but I don't want to rewrite the entire MultiRunner class: If these methods remain "private", I will have to copy-and-paste the entire source of MultiRunner into my own version of MultiRunner. Instead I would prefer to simply extend MultiRunner to add my "tick()" method which is required to have MultiRunner work on the event loop.

This works:

    time_log("Creating requests...");
    $request1 = F2\httpRequestFactory()->createRequest('GET', 'https://www.vg.no/');
    $request2 = F2\httpRequestFactory()->createRequest('GET', 'https://www.dagbladet.no/');

    time_log("Sending request 1");
    $promise1 = F2\httpClient()->sendAsyncRequest($request1);

    time_log("Sending request 2");
    $promise2 = F2\httpClient()->sendAsyncRequest($request2);

    time_log("Sleeping for 0.3 seconds to see that the requests are actually running.");
    $f = yield sleep(0.3);

    time_log("Waiting for request 2");
    $response2 = yield $promise2;
    time_log("Status code: ".$response2->getStatusCode());

    time_log("Waiting for request 1");
    $response1 = yield $promise1;
    time_log("Status code: ".$response1->getStatusCode());

Outputs:

       2 ms  Creating requests...
       4 ms  Sending request 1
       8 ms  Sending request 2
       9 ms  Sleeping for 0.3 seconds to see that the requests are actually running.
     313 ms  Waiting for request 2
     450 ms  Status code: 200
     450 ms  Waiting for request 1
     606 ms  Status code: 200
     606 ms  All done

And also you should use dependency injection instead of my service locator pattern I've heard.

[letscodehu]

@frodeborli private means you shouldn't worry about that part as a client of the lib, and they shouldn't worry about as a maintainer of the lib either. If they make it protected they will open a door which let's you (and others) extend it and depend upon that functionality. It does not sound too bad but think about how you tie their hands in case they meant to rewrite or even delete that section in the future? Maintaining open source libraries like this require a lot of dedication and hard work, let's not make this even harder for them if you can easily work around the problem like they said above.

[mike-zenith]

Hey @frodeborli !

I understand that your use-case would be solved much easier with protected methods. I have no doubt that at the moment you choose a hacky way to reach your goal.

PHP is definitely not the best OOP language and it is easier to get used to bad practices. In other programming languages, you might have the option to not expose a class at all, therefore you would not have any access to it. You would not even know it exist. When you create a library, you want to make sure no one modifies your business rules. You might need to open little doors for clients to intercept, modify, extend the flow through interfaces but other than that, you want to make as little room for mistakes as possible while having the option to refactor internals.

An interface might be missing, I can not decide yet as I dont see your code and how you wanted to use the metioned class.

We, lazy devs, tend to choose the cheapest solution to the problems we face. When we want to modify the io of a method, we usually choose the cheapest decorator pattern: extend. There are other, better ways to achieve that, like wrapping the 3rd party library, that is actually safer, future-proof and makes testing easier.

Private methods make object oriented programming a complete waste of time

I would love to have a conversation about this sentence. Encapsulation, protecting your very own object/subject and enforcing composition is quite important in engineering and not just programming. I agree that it might seem that its only there to make things harder but it also saves you in long term.

[frodeborli]

@letscodehu When you make a class that you don't want people to extend, you should mark it as a final class, and you should still use protected instead of private for your own sake. People don't seem to understand the difference between private and protected.

[frodeborli]

@mike-zenith While I respect and applaud your intention to educate other developers, I wish I had the time to persuade you guys.

In general, I prefer to design libraries in a way that allows me as the library maintainer to use inheritance if I want to. Since I usually don't know if I will ever want to in advance, I consistently declare things as protected. I only declare things private if there is a real reason to do so.

I used to declare things private methods exclusively when I started developing object oriented, but I won't go back to that ever again.

I also avoid over-engineered libraries that create factory factories and configurable configuration and composition composers and dependency injector injectors and all those things that make subtle bugs creep in everywhere.

I like this simple library and I like the design of it. It took me 5 minutes to understand it and get it to work with my event loop.

I do respect that you have other priorities when designing your library, and I hope you respect that my priorities are different - which is why I've decided to fork this library.

This is only a curl-adapter for the php-http package, so that decision was quite simple :)

[sagikazarmark]

@frodeborli let's agree to disagree. 🙂

I (as a library maintainer) have the power to make a private method protected whenever I want, so I don't really feel limited in being able to use inheritance if I want to...which I usually don't. 🙂

I have the feeling that object orientation and inheritance are very strongly connected in your mind, which (you probably noticed) the rest of the commenters (myself included) strongly disagree with.

it is a common design mistake that many programmers make

While I respect your opinion, in my eyes yours is the "wrong" and mistaken one, so it's probably a good idea to stick to facts and arguments instead of making absolute statements like that. 🙂

[frodeborli]

@sagikazarmark I have a tendency to be drawn into these discussions, when I really don't have time for it :)

I used to teach OOP at a university in Norway, and I have had this discussion many times - which is why I am reluctant to engage in it.

This library is clearly written by somebody that comes from a modular programming language, because it is using classes as a substitute for modules, and public is being treated as a substitute for declaring exports. Everything else is declared as private as a general rule of thumb.

The term for this is interface based architecture (https://en.m.wikipedia.org/wiki/Interface-based_programming).

It is not object oriented programming.

[dbu]

Thanks all for having kept this discussion an exchange of ideas and concepts and not devolve into personal insults or the like :+1:

I am closing the issue now, as the discussion is not about whether this library is useful anymore. That question seems to be answered by those who said that they use it and are glad it exists...

[sagikazarmark]

Funny thing: I can still comment on the issue, but cannot react to comments, so here it is: 👍 ❤️

[dbu]

i locked conversation, but that seems to mean that people with write access to the repository can still comment.