Question: why md5 the cache key?

[tremby]

Just out of interest, is there any particular advantage to md5ing the URL to use as a cache key at https://github.com/prismicio/php-kit/blob/master/src/Prismic/SearchForm.php#L284 ? Why not just use the URL itself as the cache key? It'd make for easier debugging, and every cache implementation is storing it in a hashmap (and therefore hashing the key) anyway. You don't pre-hash it at https://github.com/prismicio/php-kit/blob/master/src/Prismic/Api.php#L294.

[erwan]

It's to limit the length of the key: https://github.com/prismicio/php-kit/commit/9890bb7f08c573909d1c9d4d5e8dea512a5b677c

[tremby]

What does memcached have to do with it? This library doesn't provide support for memcached as far as I can see.

It seems to me that logic to get around a particular backend's limitation (neither APC, which is the only supported backend, nor Redis have such a low limit) should be dealt with by the particular cache implementation, rather than imposed on all cache types.

It's a (small but) needless performance hit on other cache backends, as it is.

[erwan]

Calculating a md5 for a string that size in PHP is a few microseconds, so if you compare that to the network latency to reach your user from your server that you can't avoid anyway, it really is negligible. I'm sure you'll have plenty of room for improvement in your code where you can shave milliseconds before having to care about these microseconds.

And it kind of make sense to limit the length of keys not just for memcached, other cache implementations may have the same limitation or benefit performance-wise from a smaller key.

[tremby]

Again, I argue that that should be the responsibility of the cache implementation, certainly not of SearchForm.

[erwan]

I understand, it's a valid argument - it could be the responsibility of the cache implementation. That said I don't see any strong issue with the way it is (performance hit is negligible) so we'll keep it this way for now to make it easier to write cache implementations.

[tremby]

I agree that the performance hit is negligible. But I'd contend that rather than making implementations easier it does the opposite, since it's harder when debugging to verify by inspection that the right values are stored in the right places.

And you're arbitrarily limiting to 32 hex characters in that one single case. There is one other case in the codebase (mentioned above) which is not limited to this size, and who is to say that this one isn't too long, or that the 32 character limit would be useful for all or the majority of potential cache backends anyway? Are API developers to know and remember that they should hash keys before making any new calls to store things in the cache to avoid upsetting any cache implementations which come to rely on this limit?

I'll stop here and move on, but I hope I've highlighted why this sort of decision only adds confusion.

[tremby]

I still feel strongly about this, so I've made a pull request.

[rileyrg]

And years later I was asking the exact same question.