JavaScript icon indicating copy to clipboard operation
JavaScript copied to clipboard

[OTHER]: Duplicate entry of sieve of eratosthenes

Open SpiderMath opened this issue 1 year ago • 19 comments

What would you like to share?

There's three entries in total for Sieve of Eratosthenes on the website, but the problem is that there's a primary one out of the three which has a majority of the implementations, including that of JS. However, there's another one which has just another JS implementation of the exact same thing, and it's named Sieve of Eratosthenes Int Array, which is both a weird name and just a random duplicate entry. Since it only had a JS entry, I wasn't sure if I should open an issue on the website repo or here, hence I thought as this was a JS repo issue I'd open it here. So, do we remove the duplicate entry from the repository?

Additional information

No response

SpiderMath avatar May 22 '24 21:05 SpiderMath

I think we can get rid of https://github.com/TheAlgorithms/JavaScript/blob/master/Maths/SieveOfEratosthenesIntArray.js; it is strictly inferior to https://github.com/TheAlgorithms/JavaScript/blob/master/Maths/SieveOfEratosthenes.js (except perhaps for being a bit simpler, at the cost of being less optimized).

appgurueu avatar May 23 '24 17:05 appgurueu

Since you've added the beginner-friendly tag, do you wish to keep this issue open for any beginners to take it up or should I do it now and push the changes? (considering we only need to remove certain files)

SpiderMath avatar May 23 '24 18:05 SpiderMath

Since you've added the beginner-friendly tag, do you wish to keep this issue open for any beginners to take it up or should I do it now and push the changes? (considering we only need to remove certain files)

Feel free to do it now

appgurueu avatar May 23 '24 19:05 appgurueu

There are three entries for the Sieve of Eratosthenes on the website, with one primary entry containing most implementations, including JavaScript. However, there's another entry named "Sieve of Eratosthenes Int Array," which is a duplicate with only a JavaScript implementation and a confusing name.

  • Remove the "Sieve of Eratosthenes Int Array" entry to avoid confusion and maintain consistency.
  • Ensure no unique features or references are lost in the process.

Please review and confirm if this is a suitable approach.

AbhishekMane06 avatar Jul 12 '24 14:07 AbhishekMane06

There are two Sieve of Eratosthenes entries in the website ( I think they are duplicate ). It returns an array of boolean which tells the numbers are true or false.

There is another one Sieve of Eratosthenes Int Array it return the Array of numbers that are primes only and it doesn't contains any non prime numbers.

I think Sieve of Eratosthenes Int Array should not be removed, while out of two Sieve of Eratosthenes entries one should be removed because I think they are duplicate. Please tell if I am in right direction.

PranjalShrivas avatar Jul 14 '24 07:07 PranjalShrivas

As I said, I think removing the "Int Array" variant is fine. Then there is only one Sieve of Eratosthenes left.

appgurueu avatar Jul 17 '24 22:07 appgurueu

There is a pr with the necessary change, but they seem to have abandoned it without removing the related tests.

raklaptudirm avatar Jul 18 '24 11:07 raklaptudirm

is this issue still on??

shah0108 avatar Oct 01 '24 05:10 shah0108

Yes.

appgurueu avatar Oct 02 '24 00:10 appgurueu

i think i got the problem, we should remove one of the implementation right? so can u tell me what user will actually require...like https://the-algorithms.com/algorithm/sieve-of-eratosthenes-int-array this will be required when the user wants all the prime number between a given range meanwhile https://the-algorithms.com/algorithm/sieve-of-eratosthenes?lang=javascript this will be required when user wants to check primality of numbers or generate prime status for boolean array .

and according to me user will definitely want 1st one more than the 2nd one...so we should remove the 2nd one...this is the problem right??

shah0108 avatar Oct 02 '24 11:10 shah0108

I think it doesn't matter much. Both variants can easily be turned into the other via postprocessing. I feel https://the-algorithms.com/algorithm/sieve-of-eratosthenes-int-array is a bit cleaner, whereas the other one might be a bit more optimized. But ultimately they both implement the core idea of the Sieve of Eratosthenes.

appgurueu avatar Oct 02 '24 19:10 appgurueu

Thank you for your input! I agree that both implementations can achieve the same goal through post-processing, and it's great to have flexibility. However, from a user-experience standpoint, we should prioritize clarity and ease of use. Most users will likely want a list of prime numbers (like in the int array implementation), which is simple and immediately useful for most practical applications. The boolean array implementation, while optimized, adds a layer of abstraction that users may not need unless they specifically want to check primality across a range of numbers. So, for simplicity and to avoid confusion, I still think we should keep just one implementation, ideally https://the-algorithms.com/algorithm/sieve-of-eratosthenes-int-array. This way, we streamline the codebase and make it more user-friendly without compromising on functionality.

What do you think about this approach?

shah0108 avatar Oct 03 '24 02:10 shah0108

and can u tell me what you are also thinking specifically...i.e. to remove one of the two or just change the name or heading?

shah0108 avatar Oct 03 '24 02:10 shah0108

Remove one of the two, possibly rename it (if we're keeping the "int array" one, we would want to rename that to just "Sieve of Eratosthenes"), consolidate the tests.

appgurueu avatar Oct 03 '24 17:10 appgurueu

Sounds good! I agree with removing one of the implementations and renaming the remaining one to "Sieve of Eratosthenes" for clarity. I'll go ahead and: ->Remove the boolean array implementation. ->Rename the int array version to simply "Sieve of Eratosthenes." ->Consolidate the tests to ensure everything works smoothly. I'll start working on this and submit a PR shortly. Let me know if there's anything else you'd like to add!?

shah0108 avatar Oct 05 '24 04:10 shah0108

I would love to contribute in this , if it open, it is my first time , can you please guide me.

sanjogbhalla16 avatar Oct 08 '24 13:10 sanjogbhalla16

Hi if this is still open maybe I could work on it. I am a beginner.

aynaash avatar Oct 08 '24 21:10 aynaash

Hey is this still available?

m3tal10 avatar Oct 10 '24 16:10 m3tal10

Hey!! please assign this to me if it's still available.

AishwaryaTatiwar avatar Oct 13 '24 19:10 AishwaryaTatiwar

Hi I like to work with this. Is it still open to work

mayurshetty15 avatar Dec 13 '24 08:12 mayurshetty15

Hii, If they issue is still open i would like to work on this. Could you please assign me so that i can work on it

AtmajMishra avatar Jan 03 '25 04:01 AtmajMishra

Hi,this wasn't assigned to me either.

On Fri, 3 Jan 2025 at 09:38, AtmajMishra @.***> wrote:

Hii, If they issue is still open i would like to work on this. Could you please assign me so that i can work on it

— Reply to this email directly, view it on GitHub https://github.com/TheAlgorithms/JavaScript/issues/1666#issuecomment-2568669577, or unsubscribe https://github.com/notifications/unsubscribe-auth/BCMPMJTOKVRM4LUD6NEI54T2IYENBAVCNFSM6AAAAABIEN5A46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRYGY3DSNJXG4 . You are receiving this because you commented.Message ID: @.***>

AishwaryaTatiwar avatar Jan 03 '25 04:01 AishwaryaTatiwar

This issue won't be assigned to anyone, open a pull request and it will be reviewed.

  • Remove the boolean array implementation.
  • Rename the int array version to simply "Sieve of Eratosthenes."
  • Consolidate the tests to ensure everything works smoothly.

raklaptudirm avatar Jan 03 '25 13:01 raklaptudirm