JSONArray does not have constructor to allocate the specified initial capacity #516
Comments
viveksacademia4git
changed the title
|
You are right. If you have a collection you are converting to a JSONArray, this constructor does the trick: https://github.com/stleary/JSON-java/blob/master/JSONArray.java#L171 However, if you are building up from scratch using an input where you can guess the size but not know an exact size, we don't expose a way to do that. We have a code reorganization going on in PR #515, but a PR to support this should be pretty straightforward. |
|
No objection to a pull request for this issue. |
|
Should I make changes? |
|
Yes, please go ahead. |
|
Thank you @stleary. |
|
@johnjaylward and @stleary If we implement this constructor then I suppose that we also have to create a method to trim the size of the private ArrayList, like trimToSize(). So the final class will have these added components:
What do you think, is it feasible? |
|
I did not know that JSON-Java-unit-test existed for the purpose of unit testing, I will pull this branch. |
|
@viveksacademia4git You have a reasonable use case for the constructor. What is your use case for the method? |
|
@stleary I do not have any practical use case as of now, but it occurred to me while working: what if someone has a use case which involves trimming the capacity of the underlying data structure. But as I said before, "is it feasible" at this moment to create this method. Or we could introduce this kind of method if someone has a genuine and practical use case, and creates an issue requesting this functionality. |
|
Recommend that you just add the constructor for now. |
|
Cool! |
Link: stleary/JSON-java#516 1. Checked with input parameter: 0 2. Checked with input parameter: positive number 3. Checked with input parameter: negative number If input parameter is negative number JSONException is thrown: JSONArray initial capacity cannot be negative.
|
@stleary I wasn't able to do the pull request that is why I preferred the fork option.
|
|
I wasn't able to run the |
|
@viveksacademia4git you will need to rebase because of the changes in project structure (or may be easier to just create an new branch and cherry-pick) once that's done you can use the compare options to create the PR: |
- Introduced JSONObject(int) constructor.
- int > Initial capacity of the underlying data structure
- Test for new introduced JSONArray(int) constructor.
1. Checked with input parameter: 0
2. Checked with input parameter: positive number
3. Checked with positive number input parameter and compared length
4. If input parameter is negative number JSONException is thrown:
JSONArray initial capacity cannot be negative.
…tParam-InitCapacity Development for stleary#516 completed with rebased repository
|
Hello @johnjaylward, I have rebased the project as suggested by you and added the previously written code into it; you may find the changes in the commit d088cf0. In addition to the above change, I have introduced the pipeline setting for this library in maven.yml and executed the pipelines (see actions). The pipeline has these stages maven: (1)compile, (2)test and (3)build for this library, and they are running successfully. I feel that it could provide an added advantage in terms of CI. I know this is not a part of this issue and I do not know whether it is required from your perspective so please let know whether
|
|
@stleary I'm OK with the addition of the maven.yml, you? |
|
@viveksacademia4git What is the use case for adding CI to this project? Also, please provide an overview of you would go about doing it. For the initial capacity PR, let's stick to the agreed-upon changes. |
|
Okay then, I will do the pull request with the agreed changes only. |
|
@viveksacademia4git can you create a separate PR that just has the maven.yml changes? |
|
Sure, I would be happy to do that. |
|
Please include text about why this change is needed and a description of the workflow from the point of view of the submitter. FYI, PR merges are on hold until the unit tests are fixed and a few other build problems are addressed. |
|
Okay, @stleary and @johnjaylward, I will create the issue once I find that the unit-test issue is resolved. |
…uctParam-InitCapacity Development for #516 completed with rebased repository
|
Closing this issue, since the code from PR is available in master branch. |
- Introduced JSONObject(int) constructor.
- int > Initial capacity of the underlying data structure
- Test for new introduced JSONArray(int) constructor.
1. Checked with input parameter: 0
2. Checked with input parameter: positive number
3. Checked with positive number input parameter and compared length
4. If input parameter is negative number JSONException is thrown:
JSONArray initial capacity cannot be negative.
The collection class like ArrayList has the constructor that accepts the number as an input to "Constructs an empty list with the specified initial capacity."
The absence of this functionality in the JSONArray will not affect the performance of the smaller applications with a fairly limited number of iteration, but the huge applications that involve a large number of iterations will result in the performance trade-off because ArrayList grows or expands numerous times.
The text was updated successfully, but these errors were encountered: