Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PreferSetEmpty,etc. seem misleading #481

Open
gawashburn opened this issue Feb 18, 2021 · 3 comments
Open

PreferSetEmpty,etc. seem misleading #481

gawashburn opened this issue Feb 18, 2021 · 3 comments

Comments

@gawashburn
Copy link

The inspection currently states

 "`Set[T]()` allocates an intermediate object. Consider `Set.empty` which returns a singleton instance without creating a new object."

Arguably the apply() method does not document this, but Set[T]() does not allocate a new object in practice in any version of Scala I have tested. So it might be clearer to say that the documentation does not make this guarantee about future versions.

@gawashburn
Copy link
Author

gawashburn commented Feb 18, 2021

Actually I guess you probably mean that a Seq is created because of the argument elems: A*, which I not what I thought it meant. So perhaps the message could clarified.

@mccartney
Copy link
Collaborator

Let's establish what's the truth first. And then I suggest we can adjust the message to reflect that.

I am checking for Scala 2.12.11. If I am reading the code correctly:

Not sure if you agree with this reasoning.

For 2.13.5 (just checking) I don't think that's the case indeed that an intermediate object is created:

  1. Set() calls Set.from() with an empty collection as an argument
  2. Set.from delegates to empty:
    https://github.com/scala/scala/blob/v2.13.5/src/library/scala/collection/immutable/Set.scala#L102
  3. Set.empty uses a singleton:
    https://github.com/scala/scala/blob/v2.13.5/src/library/scala/collection/immutable/Set.scala#L95
    which does not seem to create an intermediate object.

So without checking other versions my hunch is:

  • 2.11 - there's an intermediate object (this is what my memory tells me)
  • 2.12 - there's an intermediate object
  • 2.13 - there's no intermediate object

And the risk is that it's more sophisticated. Maybe the difference is even in the minor versions.

@gawashburn
Copy link
Author

@mccartney I don't disagree the about the creation of the intermediate object in some version. As my second comment on the issue indicates, the problem is that message could be clearer:

Consider 'Set.empty' which returns a singleton instance without creating a new object.

The way that sentence is worded makes me think a new empty Set instance is being created, which is not the case. This is because to me the most direct referent for "new object" is the Set being returned, not some allocations that may take place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants