CacheBuilder Javadoc should specify defaults

[GoogleCodeExporter]

The CacheBuilder Javadoc should specify the default values for all properties. 
It's not clear what the expected behavior is for the following cache:

CacheBuilder.newBuilder().build();

Original issue reported on code.google.com by cow...@bbs.darktech.org on 4 Jan 2012 at 3:47

[GoogleCodeExporter]

No expiration, no bound on cache size, no prepopulation, no garbage 
collection...just a basic Cache, which differs from a Map only in providing an 
atomic get-if-absent-compute operation via get(K, Callable).

Not sure how to say that, though...

Original comment by wasserman.louis on 4 Jan 2012 at 4:07

[GoogleCodeExporter]

Couldn't you mention the default on the getter of each property?

Original comment by cow...@bbs.darktech.org on 4 Jan 2012 at 7:36

[GoogleCodeExporter]

But the getters are not public methods, so that won't show up in the Javadocs.

Original comment by fry@google.com on 4 Jan 2012 at 8:18

[GoogleCodeExporter]

I wasn't even aware there _were_ getters.

Original comment by wasserman.louis on 4 Jan 2012 at 8:49

[GoogleCodeExporter]

So stick the Javadoc on the setters ;)

Original comment by cow...@bbs.darktech.org on 4 Jan 2012 at 9:39

[GoogleCodeExporter]

I dunno.  The Javadoc we'd be adding would say..."By default, the cache will 
not do this thing."  That's...not really adding much value.

Original comment by wasserman.louis on 4 Jan 2012 at 11:25

[GoogleCodeExporter]

Louis,

Each property should have a magic value that denotes the default (for example, 
maximum size of -1 means "no limit"). Then it would make sense for the Javadoc 
to say that the default value is -1, which means "no limit".

Original comment by cow...@bbs.darktech.org on 5 Jan 2012 at 4:28

[GoogleCodeExporter]

No, because the magic value isn't ever exposed. In fact, the user can't set to 
the magic value. So I'm agreeing with Louis that the default behavior for most 
of these isn't relevant. Other than concurrencyLevel, which is already 
specified. If you have a specific default you're concerned with, please specify 
which. :-)

Original comment by fry@google.com on 5 Jan 2012 at 4:30

• Changed state: WontFix

[GoogleCodeExporter]

I tend to believe you *should* expose magic values in the Builder (just in case 
the user wants to set it to one value in one method and another in another 
method). If you decide against this then please document the default behavior 
in the class-level Javadoc.

Users need to know how "CacheBuilder.newBuilder().build()" will behave.

Original comment by cow...@bbs.darktech.org on 5 Jan 2012 at 6:55

[GoogleCodeExporter]

The default for all of the options you can set for a cache using CacheBuilder 
is "not set". I think it's fairly clear: if you don't set an option, it doesn't 
have that option. CacheBuilder.newBuilder()'s Javadoc already specifies some 
defaults: strong keys, strong values, no eviction. The default concurrency 
level is also specified in that method.

As far as exposing magic values, I think that would be a bad idea. I don't 
think there's any good reason to allow setting an option you've already set to 
something else on a CacheBuilder. You can use a different CacheBuilder if you 
want different options. Even if there were a good reason, in the vast majority 
of cases one wouldn't want to do that, and so disallowing it helps programmer 
errors that might otherwise go unnoticed be caught early.

Original comment by cgdec...@gmail.com on 5 Jan 2012 at 7:15

[GoogleCodeExporter]

Er, by "that method" I mean the concurrencyLevel() method, not newBuilder().

Original comment by cgdec...@gmail.com on 5 Jan 2012 at 7:17

[GoogleCodeExporter]

Fry,

I don't understand why you closed this issue as Won't Fix. My initial request 
was for beefing up the documentation, not for adding magic values. Are you 
saying you are unwilling to add a mention of the defaults to the Javadoc?

cgdecker,

Sorry, but when I saw this class for the first time the defaults weren't 
obvious to me. I naturally assumed that any behavior a specification does not 
specify is, by definition, undefined. Normally this implies that a behavior is 
implementation-specific and is bound to change in the future. If this isn't the 
message you're trying to send out then I recommend spelling it out in the 
specification.

Original comment by cow...@bbs.darktech.org on 6 Jan 2012 at 6:17

[GoogleCodeExporter]

I marked this as Won't Fix as there has been no specific proposal as to which 
defaults need to be documented. I'm putting the burdon of suggesting which ones 
need to be changed on you. If you'd like I can change the status, but we won't 
be able to take any practical action without a more specific proposal from your 
part. :-/

Original comment by fry@google.com on 6 Jan 2012 at 2:19

• Changed state: New

[GoogleCodeExporter]

Thanks for the clarification Fry.

I recommend mentioning the default for the following methods:

expireAfterAccess
expireAfterWrite
maximumSize

I recommend omitting it for "initialCapacity" because this property impacts 
performance, not behavior, and we want to allow for future optimizations.

Original comment by cow...@bbs.darktech.org on 6 Jan 2012 at 3:49

[GoogleCodeExporter]

Then I'm back to not understanding you. The default for all 3 of those methods 
is not to expire, and not to evict. So there really isn't a default value. If 
your suggesting that we document the magic value we use internally, I strongly 
disagree. We care about documenting external behavior, not internal 
implementation details.

Original comment by fry@google.com on 6 Jan 2012 at 3:58

[GoogleCodeExporter]

Original comment by wasserman.louis on 6 Jan 2012 at 4:29

• Added labels: Package-Cache, Type-Documentation

[GoogleCodeExporter]

All I'm looking for is "(by default, entries do not expire)" for expireAfter*() 
and "(by default, entries are not evicted)" for maximumSize(). The terminology 
is in line with the other methods and does not mention any magic values.

Original comment by cow...@bbs.darktech.org on 6 Jan 2012 at 10:11

[GoogleCodeExporter]

The class javadoc already says "Entries are automatically evicted from the 
cache when any of maximumSize, maximumWeight, expireAfterWrite, 
expireAfterAccess, weakKeys, weakValues, or softValues are requested." I am 
satisfied with that being sufficiently explicit here.

Original comment by fry@google.com on 11 Jan 2012 at 12:11

• Changed state: WorkingAsIntended

[GoogleCodeExporter]

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

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

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

• Added labels: Restrict-AddIssueComment-Commit