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

Update ArrayUtil.js #1

Closed
wants to merge 3 commits into from
Closed

Update ArrayUtil.js #1

wants to merge 3 commits into from

Conversation

FreezePhoenix
Copy link

Lets just say that this might help a little?

FreezePhoenix added 2 commits July 18, 2018 18:18
Lets just say that this might help a little?
@FreezePhoenix
Copy link
Author

@fabiocaccamo This PR adds the Array.zip and Array.unzip methods you were going to add.

@fabiocaccamo
Copy link
Owner

Hi,
I appreciate your support but your PR is not ok for the following reasons:

  • as you can see, the whole lib is written in es5
  • your ArrayUtil.zip and ArrayUtil.unzip code is not clear, there are a lot of function calls, please check my implementation, it's very simple :)
  • your ObjectUtil.clone implementation doesn't clone nested objects and arrays.

By the way, I just published this lib and I'm writing tests now, many things could change in the short period...

@FreezePhoenix
Copy link
Author

Hi,
Thank you for your constructive response,
As you have noticed, ObjectUtil.clone does not clone nested objects and arrays. That is called deep-cloning. If you were to implement such a method, it would be ObjectUtil.deepClone or ObjecUtils.cloneDeep.

As for the ArrayUtil.zip and ArrayUtil.unzip I can fix that.

@FreezePhoenix
Copy link
Author

@fabiocaccamo I would like this PR to be reopened. The operations added in this PR are not 'simple' nor is there a 'best' way to do them.

fabiocaccamo pushed a commit that referenced this pull request May 7, 2024
Update isPrime in NumberUtil.js for faster performance
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

Successfully merging this pull request may close these issues.

None yet

2 participants