Hey Anatol, > Am 29.06.2015 um 22:41 schrieb Anatol Belski <anatol....@belski.net>: > > Hi Bob, > >> -----Original Message----- >> From: Bob Weinand [mailto:bobw...@hotmail.com] >> Sent: Monday, June 29, 2015 6:44 PM >> To: Dmitry Stogov >> Cc: francois; Anatol Belski; Anatol Belski; PHP Internals >> Subject: Re: [PHP-DEV] Improved zend_string API >> >> Hey, looks like I'm a bit late to the party, but I'd like to object against >> the >> accessor macros on zend_string. >> >> What does it help to replace very nice ->val, ->len etc. with macros? >> >> ZSTR_VAL(str)[ZSTR_LEN(str)] = 0; or str->val[str->len] = 0; >> zend_string_set_len(str, ZSTR_LEN(str) - 2); or str->len -= 2; Also, that >> particular >> function suggests to me that the string is expanded if lengths don't match. >> >> I much prefer direct accessors which indicate a) no side-effects happening >> here, >> b) less functions to remember (you just look at the struct and know >> everything >> about it) and c) end up being more readable. >> >> Yes, I'm totally aware that when we ever want to change the API (I don't see >> coming it in near future anyway), it could help having wrappers. But wrappers >> don't help against every API change… let's say for some reason we'd want to >> go >> back to the old char*/int; would wrappers help? no. (Yes, that's maybe a bad >> example, but just trying to show that, while it has maybe a theoretical >> gain, I >> don't see a real practical benefit.) zend_string is really a core structure >> and >> extremely simple. Please don't trade simplicity for a small other benefit. >> >> Also, we still leave zend_string struct exposed (presumably for allowing >> inlining). >> At the point where we are now, many extension authors probably already are >> also accessing zend_string directly. We're going to have a colorful mix of >> code >> using the APIs and not using it, which is the worst we can get now. >> >> To end up, I very much like unifying prefixes (ZSTR_*), but I strongly >> dislike >> hurting readability and consistency so much for such little benefit. >> > I can only express my background on accepting this patch. > > In first place, the names ZSTR_* are really speaking, like (z)end_(str)ing > and the rest mirros the name of the actual member. That is better than STR_*. > Or like IS_INTERNED vs ZSTR_IS_INTERNED, talking about what happens. With the > set len function - yep, probably a bit misleading. > > Thinking also about the new foreach macros for hash or even > zend_hash_num_elements symbol which has survived from PHP5 among others. Same > here, access to one member encapsulated. Though direct accesses > ->nNumOfElements is still here and there, where it's useful or no care taken. > Just to name one analogue. A macro doesn't cost anything, while it does its > job abstracting things. > > The PR is closed. It's not about forcing anyone to not to use ->val and alike > (maybe indeed it is), but about what one calls application programming > interface. aka API. ->val is not an API. For the outer codes it's probably > the own responsibility on using the APIs vs direct access. > > Regards > > Anatol
They're definitely speaking and as I said "I very much like unifying prefixes". That's totally fine and not what I'm complaining about. Generally, I'm really just disliking ZSTR_LEN, ZSTR_VAL(), zend_string_set_len() and zend_string_revert_hash(). Sure, but who wants to type that? nNumberOfElements? I don't feel like you can remember that very well. oh… and wait. It's called nNumOfElements… See? That's why we needed a wrapper here. Point here is that it doesn't abstract anything, it just renames it. It costs the confusion it creates. Should I directly access it or not? There it was directly accessed, but there not. If we'd done that, we should have done that at the very beginning IMHO. Not now. And still, using a struct is an API. The members have a well defined meaning what they do. The struct itself is an API, it is an interface giving you the offset from base zend_string * address. Using "my_len = *((size_t*)((char *)zend_string) + 16))" would be an example of not programming to the API. That's what a struct provides. A layer of abstraction over the offset. Hence it's already a level of abstraction. I'd like to urge you to not have two (used) APIs. And considering, that IMHO the direct accessors are much more readable than the macros and currently also much more widely used, I'd like to remove these 4 macros/inline functions. Thanks, Bob -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
