On 24 Jan 2016 19:44, "Marco Pivetta" <ocram...@gmail.com> wrote: > > On 24 January 2016 at 20:34, Jakub Zelenka <bu...@php.net> wrote: >> >> On 24 Jan 2016 14:49, "Marco Pivetta" >> > >> > The BC breaks may happen in future, given the number of added parameters. You even designed your additional parameters to avoid BC breaks: what will happen next time someone needs additional functionality? >> >> I'm not aware about any new functionality that could be added here in the near future and even if there was any, new parameters can still be added. I'm pretty sure that once this happens we will have already named params support which should address all your concerns. I actualy think that with the named params this will be much better API change than all other propasals. > > > Exactly: like anyone else, you are not yet aware about it. What happens in 5 years?
Of course I don't know what happens in 5 years but even if we needed to add new parameters, it wouldn't be a BC break. >> >> I'm sorry but I don't think we should add any objects here. OpenSSL ext is purely procedural and objects wouldn't fit. It would also require some code reordering and clean up. > > Doesn't really matter if you design it with an object or with optional parameters, or even with an array of `$options`, as long as you can enforce correct types passed in, and the order of the default parameter values doesn't become a mess (as it currently is becoming). I don't think that array would be an option as it's not realy nice when a tag would be returned into it (tag is a ref and filled after encryption)... the correct types are already enforced in the current implementation (warning is triggered if they are not correct). About the order, the only default is the last paremeter so I don't see any issue here either. As I said I really think that we will get named params before there is any need to add another param. After that the current API will be really good (just look how many parameters are often in Python libraries). >> >> >> >> That being said I know that the current proposal is not the nicest but I think it fits to the current openssl ext API which is not the nicest either. >> > >> > This is indeed an adapter: if OpenSSL sucks, then don't jump off the bridge as well ;-) >> >> I have never said that OpenSSL sucks. :) It's a great library written by one of the smartest people in the world. I meant that our openssl extension doesn't have the nicest API. However the API is more or less done in the same style which we shouldn't change IMHO. > > I write loads of stuff that may or may not suck from a consumer perspective. I am blaming the API, not the developer, let's not bring it to a personal level. Apology if it seemed that I'm bringing it on personal level. That wasn't my intention. What I was trying to say is that I think we should try to keep the extension consistent. I think that your idea is good in general but it doesn't fit to the openssl ext in my opinion. > > We are designing (changing, eek!) an API here: the fact that it may or not reflect how OpenSSL itself exposes it is totally irrelevant. The OpenSSL libcrypto API was never the point here. I might have been a bit unclear. If so, then I apologise. I actually meant other PHP functions from openssl core extension (those starting with openssl_). There are no other objects in the extension, just resources and functions with many parameters. Some of them also using refs for out params. I also think that it's more conforming with the openssl_encrypt and openssl_decrypt that got iv parameter at some later point. We don't have a new pair of functions with and without iv though... Cheers Jakub