Skip to content

Reverting toArray call, it is valid to pass in pre-alloc size - #45

Open
zsmoore wants to merge 1 commit into
devfrom
fixSize
Open

Reverting toArray call, it is valid to pass in pre-alloc size#45
zsmoore wants to merge 1 commit into
devfrom
fixSize

Conversation

@zsmoore

@zsmoore zsmoore commented Jun 5, 2019

Copy link
Copy Markdown
Owner
  • Please check if the PR fulfills these requirements
  • [N/A] Tests for the changes have been added (for bug fixes / features)
  • [ N/A] Docs have been added / updated (for bug fixes / features)

https://docs.oracle.com/javase/7/docs/api/java/util/Set.html#toArray(T[])

and If the set fits in the specified array, it is returned therein. Otherwise, a new array is allocated with the runtime type of the specified array and the size of this set.

Passing size is a valid thing to do.

  • What is the current behavior? (You can also link to an open issue here)
    Current behavior is the same as

(JsonFile[]) filesToLint.toArray()

  • What is the new behavior (if this is a feature change)?
    Array is optimized with same size as needed and no longer makes an unnecessary array of size 0 on each toArray call.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    None.

  • Other information:

@zsmoore zsmoore added the bug Something isn't working label Jun 5, 2019
@zsmoore
zsmoore requested review from PawanHegde and ankuagarwal June 5, 2019 00:48
@zsmoore zsmoore self-assigned this Jun 5, 2019

@PawanHegde PawanHegde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is valid to pass in a fixed sized array, it is also just as valid (maybe more?) to pass in a zero-sized array as a type hint when you don't want a larger array. Our intention is to get an array of exactly the same size as the set. Passing 0 ensures this. There are no side-effects as far as it concerns our code. The link you cited also uses the example String[] y = x.toArray(new String[0]); instead of String[] y = x.toArray(new String[x.size()]);.

As far as performance is concerned, new String[0] is faster as I explained in PR #40 despite the JVM needing to create a throwaway empty array. So if you're concerned about performance, us fixing the size of the array will be slower than letting the JVM decide. Not to mention the extra function call to filesToLint.size() (which will also be really fast on an optimized JVM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants