yf0994 / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Make a public Abstract(Non)StreamingHashFunction #938

Closed GoogleCodeExporter closed 9 years ago

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

Original issue reported on code.google.com by drewm...@gmail.com on 15 Mar 2012 at 6:30

GoogleCodeExporter commented 9 years ago
Any thoughts Dimitri?

Original comment by kak@google.com on 15 Mar 2012 at 6:36

GoogleCodeExporter commented 9 years ago
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

Original comment by andr...@google.com on 15 Mar 2012 at 7:52

GoogleCodeExporter commented 9 years ago
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!

Original comment by drewm...@gmail.com on 15 Mar 2012 at 8:56

GoogleCodeExporter commented 9 years ago
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.

Original comment by wasserman.louis on 15 Mar 2012 at 9:08

GoogleCodeExporter commented 9 years ago
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

Original comment by andr...@google.com on 15 Mar 2012 at 9:19

GoogleCodeExporter commented 9 years ago
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.

Original comment by drewm...@gmail.com on 15 Mar 2012 at 9:40

GoogleCodeExporter commented 9 years ago
We place a high value on making it as difficult as possible to shoot yourself 
in the foot.

Original comment by wasserman.louis on 15 Mar 2012 at 9:48

GoogleCodeExporter commented 9 years ago
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.

Original comment by kevinb@google.com on 16 Mar 2012 at 1:47

GoogleCodeExporter commented 9 years ago
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).

Original comment by andr...@google.com on 16 Mar 2012 at 2:18

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 3 Apr 2012 at 5:13

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 30 May 2012 at 7:45

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:14

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08