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

Add eratosthenes sieve method for finding primes below given number #672

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

anoopemacs
Copy link

@anoopemacs anoopemacs commented Oct 17, 2020

Description of Change

Notes: Added 'Sieve of Eratosthenes' algorithm along with test cases for primes below 100

@Panquesito7 Panquesito7 added the enhancement New feature or request label Oct 18, 2020
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Well written and documented code. 😄 👍
Please enable GitHub Actions in your repository of this fork in this link: https://github.com/anoopemacs/C/actions

* Test function
* @return void
*/
void test()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void test()
static void test()

}

/**
* Test function
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Test function
* @brief Test function

}

/**
* Driver Code
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Driver Code
* @brief Driver Code


/**
* Driver Code
* @return None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @return None
* @returns 0 on exit

@@ -0,0 +1,72 @@
/**
* @file
* @brief Get list of prime numbers using Sieve of Eratosthenes
Copy link
Member

Choose a reason for hiding this comment

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

Provide a Wikipedia link in Markdown format (if possible).
If there's no Wikipedia link, provide another web source for algorithm explanation. 🙂

Comment on lines 14 to 18
#include <assert.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Copy link
Member

Choose a reason for hiding this comment

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

Provide a brief description of the headers (what do they add).

Suggested change
#include <assert.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h> /// for assert
#include <stdbool.h> ///
#include <stdio.h> ///
#include <stdlib.h> ///
#include <string.h> ///

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Amazing work, I like how you've made the code. 😄 👍
Code and documentation is pretty good and refined; LGTM. 🙂

@Panquesito7 Panquesito7 added approved Approved; waiting for merge and removed Changes requested labels Oct 18, 2020
@Panquesito7 Panquesito7 requested a review from kvedala October 18, 2020 19:32
@anoopemacs
Copy link
Author

Thank you very much @Panquesito7 for your detailed review and guidance! I really appreciate you taking the time to guide in such detail :)

Copy link
Collaborator

@kvedala kvedala left a comment

Choose a reason for hiding this comment

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

👍 well written code. Please check the comments.

*/
bool* sieve(int N)
{
bool* primep = calloc(N+1, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

dynamically allocated memory must be freeed.
In this case, a note should be added to the function that the function free() must be called on the output pointer after its use.

For example, on line# 48, pointer to a new memory is returned. Hence, before the end of that function, there must be a free(primep);

for (size_t i = 0, size = sizeof(primers) / sizeof(primers[0]); i < size;
++i)
{
assert(primep[primers[i]]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

the correct loop check would be:

for (size_t j = 0, p = 0; j < 100; j++) 
{
  if (j == primers[p]) // j is a known prime number
  {
      assert(prime[j]);
      p++; // this variable is used to keep a track of the index of known prime numbers array
  } else { // j is a known composite number
      assert(!prime[j]);
  }
}

This will check that your function ensures that both primes and composites are classified correctly.

assert(primep[primers[i]]);
}

/* Example Non-prime numbers */
Copy link
Collaborator

Choose a reason for hiding this comment

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

good code :)
but becomes redundant after the above suggested loop. You can keep this loop as well - I like it as it is a good exercise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants