google / guava

Google core libraries for Java
Apache License 2.0
50.19k stars 10.9k forks source link

Discourage Maps.newConcurrentMap() in favor of "new ConcurrentHashMap<>()?" #2085

Open cpovirk opened 9 years ago

cpovirk commented 9 years ago

Similar to what we've done for Lists.newArrayList() and friends.

Stephan202 commented 9 years ago

I think this is a good idea, though it'd be good to warn users that (naturally) the default concurrency level may be different (see andrewgaul/modernizer-maven-plugin@6cc37bd528fbbcf6312eda58a53c307ead560ae8).

For other candidates, see the com/google/... entries in modernizer.xml.

garcia-jj commented 9 years ago

At this time Guava needs to be compatible with Java 6. Since diamond is only available in Java 7+, IMHO, I don't think this is a good idea. I think we can think in this when Guava requirements when updated to Java 7.

kevinb9n commented 9 years ago

Otávio, the proposal is only to discourage them in the similar fashion to how we already do Lists.newArrayList():

Note for Java 7 and later: this method is now unnecessary and should be treated as deprecated. Instead, use the ArrayList constructor directly, taking advantage of the new"diamond" syntax http://goo.gl/iz2Wi.

On Sun, Jun 21, 2015 at 10:16 PM, Otávio Garcia notifications@github.com wrote:

At this time Guava needs to be compatible with Java 6. Since diamond is only available in Java 7+, IMHO, I don't think this is a good idea. I think we can think in this when Guava requirements when updated to Java 7.

— Reply to this email directly or view it on GitHub https://github.com/google/guava/issues/2085#issuecomment-114008469.

Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb@google.com

cpovirk commented 9 years ago

See also my fears at https://github.com/google/guava/issues/2079#issuecomment-117189783 about Proguard problems resulting from the implementation in terms of MapMaker. Things don't seem to be as bad as I expected, but it might be enough that we should change the implementation.