Re: Improved zend_string API

Hey Anatol, > Am 29.06.2015 um 22:41 schrieb Anatol Belski <anatol.php@belski.net>: > > Hi Bob, > >> -----Original Message----- >> From: Bob Weinand [mailto:bobwei9@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