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

deepKeys works wrong for sparsed arrays #109

Closed
AuthorProxy opened this issue Jun 30, 2023 · 4 comments
Closed

deepKeys works wrong for sparsed arrays #109

AuthorProxy opened this issue Jun 30, 2023 · 4 comments

Comments

@AuthorProxy
Copy link
Contributor

AuthorProxy commented Jun 30, 2023

deepKeys should not return not existed keys

it("deepKeys should return correct values for sparsed arrays", () => {
  // fixture setup
  // eslint-disable-next-line no-sparse-arrays
  const mockedData = [1, , 3];

  const expectedState = Object.keys(mockedData).map(k => `[${k}]`);

  const sut = () => deepKeys(mockedData);

  // exercise system
  const actualResult = sut();

  // verify outcome
  expect(actualResult).toEqual(expectedState);
});
- Expected  - 0
+ Received  + 1

  Array [
    "[0]",
+   "[1]",
    "[2]",
  ]Jest

related to #108

@sindresorhus
Copy link
Owner

I think the behavior is the correct one. Sparse arrays is a misfeature and I don't plan to go through hoops to support it. My only intention was for it to not crash.


Sparse arrays in JavaScript can be useful in some cases, but there are several reasons why they might be considered disadvantageous or bad practice for many scenarios:

  1. Memory Inefficiency: While sparse arrays are often considered to save memory, in JavaScript this is not necessarily the case. Due to the dynamic nature of JavaScript arrays (which are essentially objects), the memory allocation might be inefficient compared to dense arrays.

  2. Performance Issues: Sparse arrays can lead to performance issues. Accessing elements in a sparse array tends to be slower than in a dense array because the JavaScript engine has to do more work to look up elements in the hash-map-like structure that underlies a sparse array.

  3. Complexity and Readability: Sparse arrays add an unnecessary layer of complexity and can make the code harder to understand. When another developer reads the code, they might not immediately recognize that the array is sparse and this can lead to misunderstandings or mistakes.

  4. Inconsistent Behavior with Built-in Methods: Many built-in array methods (e.g., forEach, map, etc.) behave differently with sparse arrays compared to dense arrays. This can lead to unexpected results if you're not careful. For example, forEach will skip the missing indices in a sparse array, which might not be the desired behavior.

  5. Iteration Inefficiency: Iterating over a sparse array can be inefficient. Traditional loops like the for loop will iterate through all indices, including the empty ones, which can be extremely slow for very sparse arrays. On the other hand, using methods like forEach can obscure the sparse nature of the array and lead to confusion.

  6. Cross-browser and Cross-version Inconsistencies: There might be inconsistencies in the behavior of sparse arrays across different browsers or JavaScript engine versions. This can make it difficult to write robust, cross-platform code.

  7. Error Prone: It is easy to accidentally create a sparse array, for instance, by inadvertently setting an element at a high index. This can lead to unexpected behavior and bugs that are difficult to track down.

  8. Serialization Issues: When converting a sparse array to JSON using JSON.stringify, the sparse array is converted to a dense array, with null values filling in the gaps. This can cause unexpected behavior if the array is serialized and then deserialized.

  9. Type Inconsistencies: When working with typed arrays or expecting all elements in an array to be of a certain type, sparse arrays can introduce type inconsistencies, as the undefined slots are neither of any type nor do they hold any meaningful value.

  10. Community and Best Practices: Sparse arrays are not commonly used in the JavaScript community. As such, using them may go against established best practices and community conventions, making your code harder to maintain by others or integrate with libraries and frameworks.

Given these reasons, it is often recommended to use alternative data structures like sets, maps, or objects when you need to store non-sequential keys, as they do not suffer from the same downsides as sparse arrays in JavaScript.

@sindresorhus sindresorhus closed this as not planned Won't fix, can't repro, duplicate, stale Jun 30, 2023
sindresorhus added a commit that referenced this issue Jun 30, 2023
@AuthorProxy
Copy link
Contributor Author

AuthorProxy commented Jun 30, 2023

@sindresorhus
I know that there some minuses, but for my situation it is pluses and expected behaviour. I don't care about memory or cpu performance, because iterating not existed keys, double checking after 'deepKeys' wrong work or creating a new structures is much more heavy and complex for my task.

Anyway deepKeys should return only existed keys, it cannot work opposite to object.keys([1,,3]) in my opinion. Now this behaviour leads to some bugs, I got a keys which I never set, I need to make double checking at code for each property to be sure that deepKeys would not return inexistent properties.

So if I fix this, will you merge it??

@sindresorhus
Copy link
Owner

So if I fix this, will you merge it??

Yes

@AuthorProxy
Copy link
Contributor Author

So if I fix this, will you merge it??

Yes

check #110

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