Make a public Abstract(Non)StreamingHashFunction

[gissuebot]

Original issue created by drewmark on 2012-03-15 at 06:30 PM

---

Please make AbstractStreamingHashFunction and AbstractNonStreamingHashFunction more visible (like protected or public) so that they can be extended by guava users.

[gissuebot]

Original comment posted by kak@google.com on 2012-03-15 at 06:36 PM

---

Any thoughts Dimitri?

---

Status: Acknowledged
Owner: andreou@google.com
Labels: Type-Enhancement, Package-Hash

[gissuebot]

Original comment posted by andreou@google.com on 2012-03-15 at 07:52 PM

---

I guess the reason that these are hidden is because they trigger an interesting question: What hash function do you try to implement? We would like to have a more comprehensive offering/variety of algorithms, perhaps you have a useful suggestion to make?

The practical reason was that designing for inheritance is just hard, and we (or I) were a bit lazy to properly do it. This may me amended in the future, of course

[gissuebot]

Original comment posted by drewmark on 2012-03-15 at 08:56 PM

---

I was thinking of implementing one or more locality-sensitive-hashing algorithms. Either way, it seems like you should allow others to easily extend what you've done. Doing so will encourage others to use your framework and will allow them to implement algorithms that may not be general-purpose enough to warrant inclusion. Thanks!

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-15 at 09:08 PM

---

The question, I suppose, is whether or not extensibility of the hashing utilities is a sufficiently common need or desire to merit the nonnegligible effort involved in redesigning and maintaining the package for inheritance.

[gissuebot]

Original comment posted by andreou@google.com on 2012-03-15 at 09:19 PM

---

I rather disagreed with even exposing HashCode's constructors, at least at this stage. I mean, right now, HashCode means that it was produced by one of the high-quality algorithms we expose. When it is a free for all (and one can say, e.g., HashCodes.forInt(5)), this meaning will be dilluted. But perhaps it doesn't matter much

[gissuebot]

Original comment posted by drewmark on 2012-03-15 at 09:40 PM

---

What I'm suggesting doesn't require redesigning the package (read API) for inheritance. I'm suggesting that implementors be given the option of extending these abstract classes. Nothing about the code would force anyone to use them if they preferred to implement the entire interface themselves (or needed to do so for some reason). This avoids much of what people deem "bad" about inheritance. If you disagree, please spell out what's hard and/or concerning about this.

As for making it a free for all, it goes without saying that if you implement your own anything in a dumb way you're going to get dumb results (even if you use it with a high-quality library). I don't think that argument has much merit.

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-15 at 09:48 PM

---

We place a high value on making it as difficult as possible to shoot yourself in the foot.

[gissuebot]

Original comment posted by kevinb@google.com on 2012-03-16 at 01:47 AM

---

I'm not sure where you got that impression, Louis. Two of my frequent utterances are

"It's much more important to make it easy to do the right thing, than to make it hard to do the wrong thing."

and

"Don't get into the business of manufacturing kevlar boots."

Extendable classes are just fragile and pain to document, and become harder to change. It'll take some thought. I'm inclined to do it though.

[gissuebot]

Original comment posted by andreou@google.com on 2012-03-16 at 02:18 AM

---

The big task here is AbstractStreamingHashFunction#AbstractStreamingHasher. And that relies on ByteBuffer, so exposing these classes, would either mean committing to it, or warning users with big bold font that we may break their implementations in the future (or keep this as dead code, if we ever move away from it).

I don't think anyone of us is very confident to relying on ByteBuffer forever. The only thing going on for it is that it was (relatively) easy to implement. And let's not forget we still have some performance work to do there, which is not entirely clear whether it can be done without breaking subclasses or not (we might be able to get away with it, but someone has to work the details out to know for sure).

[gissuebot]

Original comment posted by kevinb@google.com on 2012-04-03 at 05:13 PM

---

(No comment entered for this change.)

---

Status: Accepted

[gissuebot]

Original comment posted by kevinb@google.com on 2012-05-30 at 07:45 PM

---

(No comment entered for this change.)

---

Labels: -Type-Enhancement, Type-Addition

[devinrsmith]

Looks like not much has progressed on this issue for a couple years? Would be nice to more easily implement the HashFunction interface as an end user. Specifically, I'd like to be able to use it with "HmacSHA1", which would put it in a similar league with MessageDigestHashFunction (but with a key, so if implemented guava side it might look something like Hashing.hmacsha1(byte[] key))

[vorburger]

FTR: This (ancient) issue appears to be an exact duplicate of the (more recently opened) #5990. Close as dupe?

[cpovirk]

I think that the Abstract* classes could be useful for more than just the MessageDigest-backed implementation that #5990 is about, couldn't they? (I think I see a BouncyCastle-backed implementation, for example.)

[vorburger]

Oh, I see; right, make sense - so #5990 specifically for a MessageDigest "adapter," whereas this more open & general, for even from scratch implementation or (more realistically) BouncyCastle or any other such library (or e.g. some I collected and listed on https://github.com/multiformats/java-multihash/pull/42/files just the other day).

Would any contributions for this (more than #5990) be of any interest and (after review) possibly welcome?

[cpovirk]

Historically, the concern has been that we didn't carefully design these helper classes, just do whatever was convenient for our own implementations at the time. If we were to make them public, we'd want to review them more holistically, since we'd prefer not to make incompatible changes to them in the future (especially nowadays). That's been hard to prioritize that for a small number of potential users.

It's always possible that our convictions have worn down with the passage of time: Maybe the classes are good enough as they are now, and maybe we'll realistically never even have reason to change them for our own purposes. Maybe @kluever or someone else more familiar with the package has thoughts.

In fairness, I do note that AbstractStreamingHashFunction, at least, was deleted at some point. On the one hand, that was over seven years ago :) On the other hand, it was for purposes of optimizing for Android, so we could always imagine that some other constraint might pop up someday (e.g., involving Kotlin), even if the classes were to have remained untouched for a decade. I still kind of doubt that we'd need to make significant changes. And even if we wanted to, we could probably just clone the classes, make the new ones package-private "again," and modify as needed. It's just another reason that we'd need a clear push to focus on the review necessary to make this happen.