# want an Object-oriented interface for HashContext - Thread: https://externals.io/message/112034 - Original archive: https://news-web.php.net/php.internals/112034 - Started by Hans Henrik Bergan on 2020-10-12T11:49:13+00:00 - Messages: 14 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #112034 — Hans Henrik Bergan — 2020-10-12T11:49:13+00:00 URL: https://externals.io/message/112034#112034 something like $result = (new HashContext("SHA1"))->update($str1)->update($str2)->final(); (userland sample imp: https://3v4l.org/lXd3u ) I tried asking on the bugtracker ( https://bugs.php.net/bug.php?id=80221 ) , but was told to ask on this mailing list instead. --- ## #112035 — Christoph M. Becker — 2020-10-12T12:55:32+00:00 URL: https://externals.io/message/112034#112035 · In reply to: #112034 > something like > > $result = (new HashContext("SHA1"))->update($str1)->update($str2)->final(); > > (userland sample imp: https://3v4l.org/lXd3u ) > > I tried asking on the bugtracker ( https://bugs.php.net/bug.php?id=80221 ) , > but was told to ask on this mailing list instead. Thanks for bringing this up on the mailing list! I basically very much support a proper OOP interface, but I think the method names should use camel-case (e.g. ::updateFile() instead of ::update_file()), and it might be appropriate to rename ::final() to ::finalize(). More bikeshedding regarding the method names, and maybe their signatures might be in order. We do not necessarily have to make these methods aliases of the existing functions, although that's of course possible. -- Christoph M. Becker --- ## #112037 — Kingsquare.nl - Robin Speekenbrink — 2020-10-12T14:01:20+00:00 URL: https://externals.io/message/112034#112037 · In reply to: #112035 Op ma 12 okt. 2020 om 14:55 schreef Christoph M. Becker : > > > > […] > HashContext("SHA1"))->update($str1)->update($str2)->final(); > > […] > ) , > > […] > > Thanks for bringing this up on the mailing list! > > I basically very much support a proper OOP interface, but I think the > method names should use camel-case (e.g. ::updateFile() instead of > ::update_file()), and it might be appropriate to rename ::final() to > ::finalize(). More bikeshedding regarding the method names, and maybe > their signatures might be in order. We do not necessarily have to make > these methods aliases of the existing functions, although that's of > course possible. > > Dear all, As a user/developer of the language itself: it seems to me this is what a package using composer could easily fix right? Does this really have to land in core to be used in the wild? With the proliferation of all the up and coming features and performance improvements it seems to me like these types of improvements using lower level PHP interfaces / native functions have no merit in core itself? (packagist / composer packages don't always have to be full frameworks / large solutions right IMHO) This would also allow the developer to pick and choose a personal style and we don't have to flood the internals with bikeshedding ;) Robin Speekenbrink --- ## #112038 — G. P. B. — 2020-10-12T14:20:10+00:00 URL: https://externals.io/message/112034#112038 · In reply to: #112037 On Mon, 12 Oct 2020 at 15:01, Kingsquare.nl - Robin Speekenbrink < robin@kingsquare.nl> wrote: > Op ma 12 okt. 2020 om 14:55 schreef Christoph M. Becker > […] > > > […] > https://bugs.php.net/bug.php?id=80221 > > […] > Dear all, > > As a user/developer of the language itself: it seems to me this is what a > package using composer could easily fix right? Does this really have to > land in core to be used in the wild? With the proliferation of all the up > and coming features and performance improvements it seems to me like these > types of improvements using lower level PHP interfaces / native functions > have no merit in core itself? > (packagist / composer packages don't always have to be full frameworks / > large solutions right IMHO) > > This would also allow the developer to pick and choose a personal style and > we don't have to flood the internals with bikeshedding ;) > > Robin Speekenbrink > The reason for the non existence of OO APIs is that until recently these objects were resources, HashContext is only an object since PHP 7.2. [1] We performed a bunch of resources to opaque object conversion in PHP 8.0 and the plan is to migrate *all* resources to objects. [2] That's the reason why the procedural API exists, and as most of these opaque objects are marked as final (among other things) to be drop-in replacements of resources while the design of the OO API is left at a later time. Therefore I am not sure what composer and a userland package can bring when one potential avenue to explore is introducing a more thought out and/or (possibly) better API which was hindered by the technical limitation of using a resource. This can also lead us to deprecate the procedural API in favour of said new OO API. This does not need to happen for *all* resource to object conversions but it's something to keep in mind. Moreover, I prefer to have the API be part of the relevant extension than tugged away in some random composer package. Best George P. Banyard [1] https://wiki.php.net/rfc/hash-context.as-resource [2] https://github.com/php/php-tasks/issues/6 --- ## #112039 — Rowan Tommins — 2020-10-12T14:30:17+00:00 URL: https://externals.io/message/112034#112039 · In reply to: #112038 > Therefore I am not sure what composer and a userland package can bring... > As an example of this general approach, the MongoDB extension provides a minimal set of low-level "driver" functionality, and the official userland package builds on top of these to create a richer set of APIs. That means the majority of development takes place in the userland library, making it easier to maintain, easier for users to keep up to date, and so on. That's not always the appropriate approach, though - some built-in functionality is useful precisely because it's "batteries included", like the password_* functions. I'm not that familiar with the hash functions, so can't really comment whether I would expect them to be usable "out of the box" or just as primitives for a higher-level wrapper. Regards, -- Rowan Tommins [IMSoP] --- ## #112043 — Hans Henrik Bergan — 2020-10-13T09:43:38+00:00 URL: https://externals.io/message/112034#112043 · In reply to: #112039 i know this is unrelated to the thread at hand, but am i really supposed to find the email of everyone who has replied to a thread, and add them all manually to cc? (or is it sufficient to just send it to internals@list.php.net ? ) (also funfact, it seems if you regex the innerHTML after opening an email on gmail.com , the innerHTML contains *every single email you've ever interacted with*, i found 680 emails when checking the innerHTML for this thread, quite the surprise) @Christoph M. Becker > I think the > method names should use camel-case (e.g. ::updateFile() instead of > ::update_file()) that's fine by me (could mention that PHP already use both approaches, examples: procedural: numfmt_format_currency OO: NumberFormatter::formatCurrency procedural: MySQLi::affected_rows OO: `mysqli_affected_rows()` ) > it might be appropriate to rename ::final() to > ::finalize() that's fine by me (personally i'd prefer a `str()` function which didn't finalize anything, but i guess that's a separate issue altogether, besides, userland users can work around it with `hash_copy()` when it's really needed) @robin > As a user/developer of the language itself: it seems to me this is what a > package using composer could easily fix right? Does this really have to > land in core to be used in the wild? With the proliferation of all the up > and coming features and performance improvements it seems to me like these > types of improvements using lower level PHP interfaces / native functions > have no merit in core itself? > (packagist / composer packages don't always have to be full frameworks / > large solutions right IMHO) > > This would also allow the developer to pick and choose a personal style and > we don't have to flood the internals with bikeshedding ;) yes a composer package could easily wrap the current api, and it doesn't have to be done in core, but i would prefer it to be in the core, and the current HashContext class looks.. underdeveloped/underutilized/untapped/unfinished/awkward (don't know what the correct word is, English isn't my native language, but something like that) but as G. P. B. mentioned below, there's a reason for that: it used to be a resource. @ G. P. B. > The reason for the non existence of OO APIs is that until recently these objects > were resources, HashContext is only an object since PHP 7.2. [1] interesting > We performed a bunch of resources to opaque object conversion in PHP 8.0 and the plan > is to migrate *all* resources to objects. [2] > That's the reason why the procedural API exists, and as most of these opaque objects are > marked as final (among other things) to be drop-in replacements of resources while the design > of the OO API is left at a later time. neat, TIL (that leaves the question of what happens to `is_resource()` and all the code using it, but that's a different topic altogether ^^ ) > Therefore I am not sure what composer and a userland package can bring when one potential > avenue to explore is introducing a more thought out and/or (possibly) better API which was hindered > by the technical limitation of using a resource. in this case i think some version of the OO api would be better in every way (since even with the current procedural api, you still need to carry around the HashContext object anyway, i can't think of any good reason it shouldn't have the functionality embedded as methods, instead of using 5 global functions to manipulate it.. then again, maybe the same can be said for nearly all resources) > This can also lead us to deprecate the procedural API in favour of said new OO API. if there is interest in deprecating the procedural one, i guess it'll have to start throwing `E_DEPRECATED` in some 8.x release, and be removed in 9.0.0? @Rowan Tommins > I'm not that familiar with the hash functions, so > can't really comment whether I would expect them to be usable "out of the > box" or just as primitives for a higher-level wrapper. i'd expect it to be "usable out of the box" (and it is, but...clunky) > > > > > […] > > > As an example of this general approach, the MongoDB extension provides a > minimal set of low-level "driver" functionality, and the official userland > package builds on top of these to create a richer set of APIs. That means > the majority of development takes place in the userland library, making it > easier to maintain, easier for users to keep up to date, and so on. > > That's not always the appropriate approach, though - some built-in > functionality is useful precisely because it's "batteries included", like > the password_* functions. I'm not that familiar with the hash functions, so > can't really comment whether I would expect them to be usable "out of the > box" or just as primitives for a higher-level wrapper. > > Regards, > -- > Rowan Tommins > [IMSoP] --- ## #112044 — Rowan Tommins — 2020-10-13T12:04:04+00:00 URL: https://externals.io/message/112034#112044 · In reply to: #112043 On Tue, 13 Oct 2020 at 10:43, Hans Henrik Bergan wrote: > i know this is unrelated to the thread at hand, but am i really > supposed to find the email of everyone who has replied to a thread, > and add them all manually to cc? > Nope, just the mailing list address should be fine; everyone sending to the list should also be subscribed to it. Indeed, I tend to remove all CCs from my replies whenever I remember. I suspect the reason you'll see a lot of CCs is that most mail clients only have "reply" (which would often miss the list completely) and "reply to all" (which accumulates CCs), and no "reply to list" (which would pick up the list address from an appropriate header and ignore everything else). Regards, -- Rowan Tommins [IMSoP] --- ## #112045 — Hans Henrik Bergan — 2020-10-13T15:33:32+00:00 URL: https://externals.io/message/112034#112045 · In reply to: #112044 PS there might be some good reason for the `hash_final()` api really being final that i'm not aware of (not that i'm aware of any reason at all, pun not intended) also the sample class in the 3v4l link has an issue i hadn't thought of: i wanted to support method chaining, so i made nearly everything return `:self` , but `hash_update_stream()` returns int, the number of bytes actually read from the stream. that could easily be fixed with a `updateStream($stream, int $length = -1, int &$bytesRead = null):self` or something tho > > On Tue, 13 Oct 2020 at 10:43, Hans Henrik Bergan > wrote: > > > […] > > > Nope, just the mailing list address should be fine; everyone sending to the > list should also be subscribed to it. Indeed, I tend to remove all CCs from > my replies whenever I remember. > > I suspect the reason you'll see a lot of CCs is that most mail clients only > have "reply" (which would often miss the list completely) and "reply to > all" (which accumulates CCs), and no "reply to list" (which would pick up > the list address from an appropriate header and ignore everything else). > > Regards, > -- > Rowan Tommins > [IMSoP] --- ## #112046 — Rowan Tommins — 2020-10-13T15:43:39+00:00 URL: https://externals.io/message/112034#112046 · In reply to: #112045 On Tue, 13 Oct 2020 at 16:33, Hans Henrik Bergan wrote: > also the sample class in the 3v4l link has an issue i hadn't thought > of: i wanted to support method chaining, so i made nearly everything > return `:self` , but `hash_update_stream()` returns int, the number of > bytes actually read from the stream. that could easily be fixed with a > `updateStream($stream, int $length = -1, int &$bytesRead = null):self` > or something tho > I think I'd prefer to drop the fluent interface (returning void on most things, and relevant values on others) than have to use reference parameters. Since PHP doesn't have actual "out" parameters, references like that are awkward for static analysis; and simple things like using them in conditions becomes much more verbose: $result = null; $foo->updateStream($bar, bytesRead: $result) if ( $result > 42 ) ... vs if ( $foo->updateStream($bar) > 42 ) ... Regards, -- Rowan Tommins [IMSoP] --- ## #112048 — Sara Golemon — 2020-10-13T16:01:12+00:00 URL: https://externals.io/message/112034#112048 · In reply to: #112035 On Mon, Oct 12, 2020 at 7:55 AM Christoph M. Becker wrote: > > > > > […] HashContext("SHA1"))->update($str1)->update($str2)->final(); > > […] ) , > > […] > > Thanks for bringing this up on the mailing list! > > I basically very much support a proper OOP interface, but I think the > method names should use camel-case (e.g. ::updateFile() instead of > ::update_file()), and it might be appropriate to rename ::final() to > ::finalize(). More bikeshedding regarding the method names, and maybe > their signatures might be in order. We do not necessarily have to make > these methods aliases of the existing functions, although that's of > course possible. > Ditto all this. When we converted resource to object I fully intended to propose adding methods on the context using pretty much exactly the API you describe. Digests lend themselves quite well to this, tbqh. I did want to separate the tech debt of removing resources from the feature creep of adding an OOP API though, and with that release already quite featurefull, plans to expand it fell by the wayside. Happy to co-author an RFC with you if you'd like to get involved directly, or I can just pick up the ball and run with it if you'd rather make the feature request then step back. -Sara --- ## #112050 — Christoph M. Becker — 2020-10-13T16:42:30+00:00 URL: https://externals.io/message/112034#112050 · In reply to: #112048 > On Mon, Oct 12, 2020 at 7:55 AM Christoph M. Becker > wrote: > > […] > HashContext("SHA1"))->update($str1)->update($str2)->final(); > > […] > ) , > > […] > > Ditto all this. When we converted resource to > object I fully intended to propose adding methods on the > context using pretty much exactly the API you describe. Digests lend > themselves quite well to this, tbqh. I did want to separate the tech debt > of removing resources from the feature creep of adding an OOP API though, > and with that release already quite featurefull, plans to expand it fell by > the wayside. > > Happy to co-author an RFC with you if you'd like to get involved directly, > or I can just pick up the ball and run with it if you'd rather make the > feature request then step back. Please go with this, if you like. Note that I didn't make the feature request; I just support it. :) -- Christoph M. Becker --- ## #112052 — Sara Golemon — 2020-10-13T17:04:37+00:00 URL: https://externals.io/message/112034#112052 · In reply to: #112050 On Tue, Oct 13, 2020 at 11:42 AM Christoph M. Becker wrote: > > > […] > directly, > > […] > > Please go with this, if you like. Note that I didn't make the feature > request; I just support it. :) > > Sorry for confusion. That part of my reply was aimed at Hans, but I didn't have his original message in my thread to reply to. :p -Sara --- ## #112072 — Sara Golemon — 2020-10-17T02:52:03+00:00 URL: https://externals.io/message/112034#112072 · In reply to: #112052 > On Tue, Oct 13, 2020 at 11:42 AM Christoph M. Becker > wrote: > > > […] > Sorry for confusion. That part of my reply was aimed at Hans, but I didn't > have his original message in my thread to reply to. :p > > No reply from Hans, so I've thrown together an implementation and will attach an RFC to it in the coming days. https://github.com/php/php-src/pull/6347 -Sara --- ## #112091 — Hans Henrik Bergan — 2020-10-21T18:49:16+00:00 URL: https://externals.io/message/112034#112091 · In reply to: #112072 @Rowan Tommins >I think I'd prefer to drop the fluent interface (returning void on most > things, and relevant values on others) well both approaches has pros and cons, that said, the sample implementation Sara posted returns $this, if you (or anyone) feels strongly about it one way or the other, i guess the php devs can vote on it @Sara Golemon thanks! (and you're the original pecl hash author, neat) given the "PHP8 release manager" thing, i guess you have a lot of other PHP8 stuff to focus on right now > Happy to co-author an RFC with you if you'd like to get involved directly, or I can just pick up the ball and run with it if you'd rather make the feature request then step back. stepping back sounds good to me, i have no idea how to do any of that stuff ^^ > No reply from Hans, so I've thrown together an implementation and will attach an RFC to it in the coming days. > https://github.com/php/php-src/pull/6347 looks great to me, and you even took care of the hash_update_stream return value issue ^^ > > > > […] > > No reply from Hans, so I've thrown together an implementation and will attach an RFC to it in the coming days. > https://github.com/php/php-src/pull/6347 > > -Sara