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

Implementing Factorial using recursion #1370

Closed
wants to merge 11 commits into from
76 changes: 76 additions & 0 deletions math/factorial_recursion.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
Algorithm: Factorial Calculation using Memoization

This C program calculates factorials of non-negative integers using a recursive algorithm with memoization.
Factorial of a non-negative integer 'n' is denoted as 'n!' and is defined as the product of all positive integers
less than or equal to 'n'. For example, 5! = 5 x 4 x 3 x 2 x 1 = 120.

Author: Sricharan Nibhanupudi

Uses:
- The program provides a function 'computeFactorial' that calculates the factorial of a given non-negative integer
using recursive memoization. It stores previously computed factorials in an array 'factResults' to avoid redundant
calculations, which significantly improves performance for large values of 'n'.
- The 'testFactorial' function uses assertions to verify the correctness of the 'computeFactorial' function by
comparing its results with known factorial values.
- The program also demonstrates the use of standardized integer data types like 'uint64_t' from 'stdint.h' for
improved code portability and readability.

References:
- The concept of factorial: https://en.wikipedia.org/wiki/Factorial
- Memoization: https://www.geeksforgeeks.org/memoization-1d-2d-and-3d/
*/

#include <stdio.h>

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be preferable if you document the headers as well

#include<header.h> /// <reason for using header>

Copy link
Author

Choose a reason for hiding this comment

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

I have made all the required changes. Kindly look into it. Thank you.

Copy link
Author

Choose a reason for hiding this comment

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

@realstealthninja, can you please review the pull request?

#include <stdint.h>
#include <assert.h>

#define MAX_LIMIT 200
Copy link
Member

Choose a reason for hiding this comment

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

Why 200? What is the highest input, for which the factorial will fit into uint64_t?


uint64_t factResults[MAX_LIMIT] = {0};
Copy link
Member

Choose a reason for hiding this comment

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

Why not to make this a static variable inside computeFactorial?


uint64_t computeFactorial(int sequenceNumber) {
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
uint64_t computeFactorial(int sequenceNumber) {
uint64_t computeFactorial(unsinged sequenceNumber) {

It does not make sense to run computeFactorial(-1), right? Why not to use unisigned as input type?

You can also go for:

uint64_t computeFactorial(const unsinged sequenceNumber) {

if (sequenceNumber == 0) {
return 1;
}
Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

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

You are wasting one slot in the factResults.

if (factResults[sequenceNumber] != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

What will happen if you call computeFactorial(1000) or computeFactorial(-1000)?

return factResults[sequenceNumber];
}
factResults[sequenceNumber] = sequenceNumber * computeFactorial(sequenceNumber - 1);
Copy link
Member

Choose a reason for hiding this comment

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

You have a conversion to 'uint64_t' from 'int' may change the sign of the result - this might be much more serious than it looks at first. Either make sequenceNumber unsinged or change this line to:

factResults[sequenceNumber] = (uint64_t)(sequenceNumber) * computeFactorial(sequenceNumber - 1);

return factResults[sequenceNumber];
}

void testFactorial() {
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 testFactorial() {
void testFactorial(void) {

cf. -Wstrict-prototypes.

assert(computeFactorial(0) == 1);
assert(computeFactorial(1) == 1);
assert(computeFactorial(2) == 2);
assert(computeFactorial(3) == 6);
assert(computeFactorial(4) == 24);
assert(computeFactorial(5) == 120);
assert(computeFactorial(6) == 720);
assert(computeFactorial(7) == 5040);
assert(computeFactorial(8) == 40320);
assert(computeFactorial(9) == 362880);
assert(computeFactorial(10) == 3628800);

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

all functions must be documented this is because when doxygen generates documentation
it pulls from your inline documentation

refer to the format below as well as the suggestion:

/*
 * @brief <explanation of function in a sentence>
 * @param <parameter name> <explanation of parameter>
 * @returns <return type>
 */
 type identifier(type param_name) {...}
Suggested change
/*
* @brief main function
* @returns 0 on successful exit
*/

Copy link
Author

Choose a reason for hiding this comment

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

Made all the required changes. Thank you.

int main() {

This comment was marked as resolved.

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
int main() {
int main(void) {

testFactorial();

int testNumber;
testNumber = 5;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
Copy link
Member

Choose a reason for hiding this comment

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

If you want to be super correct, then change this line to:

Suggested change
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
printf("The factorial of %d is %" PRIu64 "\n", testNumber, computeFactorial(testNumber));

Make sure to:

#include <inttypes.h>

testNumber = 4;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
testNumber = 6;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
testNumber = 7;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
testNumber = 9;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));
testNumber = 10;
printf("The factorial of %d is %llu\n", testNumber, computeFactorial(testNumber));

return 0;
}
140 changes: 69 additions & 71 deletions sorting/radix_sort.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this file from this pr

Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why are you deleting this file?

Original file line number Diff line number Diff line change
@@ -1,71 +1,69 @@
#include <stdio.h>

int largest(int a[], int n)
{
int large = a[0], i;
for (i = 1; i < n; i++)
{
if (large < a[i])
large = a[i];
}
return large;
}

void RadixSort(int a[], int n)
{
int bucket[10][10], bucket_count[10];
int i, j, k, remainder, NOP = 0, divisor = 1, large, pass;

large = largest(a, n);
printf("The large element %d\n", large);
while (large > 0)
{
NOP++;
large /= 10;
}

for (pass = 0; pass < NOP; pass++)
{
for (i = 0; i < 10; i++)
{
bucket_count[i] = 0;
}
for (i = 0; i < n; i++)
{
remainder = (a[i] / divisor) % 10;
bucket[remainder][bucket_count[remainder]] = a[i];
bucket_count[remainder] += 1;
}

i = 0;
for (k = 0; k < 10; k++)
{
for (j = 0; j < bucket_count[k]; j++)
{
a[i] = bucket[k][j];
i++;
}
}
divisor *= 10;

for (i = 0; i < n; i++) printf("%d ", a[i]);
printf("\n");
}
}

int main()
{
int i, n, a[10];
printf("Enter the number of elements :: ");
scanf("%d", &n);
printf("Enter the elements :: ");
for (i = 0; i < n; i++)
{
scanf("%d", &a[i]);
}
RadixSort(a, n);
printf("The sorted elements are :: ");
for (i = 0; i < n; i++) printf("%d ", a[i]);
printf("\n");
return 0;
}
#include <stdio.h>

// To find the largest number in the array
int findLargestNum(int data[], int length) {
int largest = data[0];
for (int i = 1; i < length; i++)
if (data[i] > largest)
largest = data[i];
return largest;
}

// To perform counting sort on the array based on the specified digit
void performCountSort(int data[], int length, int place) {
int output[length];
int i, freq[10] = {0};
for (i = 0; i < length; i++)
freq[(data[i] / place) % 10]++;
for (i = 1; i < 10; i++)
freq[i] += freq[i - 1];
for (i = length - 1; i >= 0; i--) {
output[freq[(data[i] / place) % 10] - 1] = data[i];
freq[(data[i] / place) % 10]--;
}
for (i = 0; i < length; i++)
data[i] = output[i];
}

// Main function for Radix Sort
void performRadixSort(int data[], int length) {
int largestNum = findLargestNum(data, length);
for (int place = 1; largestNum / place > 0; place *= 10)
performCountSort(data, length, place);
}
void displayArray(int data[], int length) {
for (int i = 0; i < length; i++)
printf("%d ", data[i]);
printf("\n");
}
int main() {
int array1[] = {121, 432, 564, 23, 1, 45, 788};
int length1 = sizeof(array1) / sizeof(array1[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

you are using sizeof(array1[0) which is a constant since its just the size of the int
so you could just do sizeof(int)

Suggested change
int length1 = sizeof(array1) / sizeof(array1[0]);
int length1 = sizeof(array1) / sizeof(int);

int array2[] = {95, 90, 85, 80, 75, 70};
int length2 = sizeof(array2) / sizeof(array2[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int length2 = sizeof(array2) / sizeof(array2[0]);
int length2 = sizeof(array2) / sizeof(int);

int array3[] = {333, 287, 911, 462, 786};
int length3 = sizeof(array3) / sizeof(array3[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int length3 = sizeof(array3) / sizeof(array3[0]);
int length3 = sizeof(array3) / sizeof(int);

int array4[] = {5, 3, 8, 1, 2, 9, 4, 7, 6};
int length4 = sizeof(array4) / sizeof(array4[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int length4 = sizeof(array4) / sizeof(array4[0]);
int length4 = sizeof(array4) / sizeof(int);

int array5[] = {12, 34, 12, 45, 67, 45, 34};
int length5 = sizeof(array5) / sizeof(array5[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int length5 = sizeof(array5) / sizeof(array5[0]);
int length5 = sizeof(array5) / sizeof(int);


Copy link
Collaborator

Choose a reason for hiding this comment

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

if you are going for speed with the test of this algorithm you could just do

float size_of_int = 1/sizeof(int);
// then multiply this value instead of divding over and over again
// remember multiplication is always faster than division
int length1 = sizeof(array1) * size_of_int;
int length2 = sizeof(array2) * size_of_int;
int length3 = sizeof(array3) * size_of_int;
int length4 = sizeof(array4) * size_of_int;
int length5 = sizeof(array5) * size_of_int;

// Sorting and displaying each array
performRadixSort(array1, length1);
printf("Sorted Array 1: ");
displayArray(array1, length1);
performRadixSort(array2, length2);
printf("Sorted Array 2: ");
displayArray(array2, length2);
performRadixSort(array3, length3);
printf("Sorted Array 3: ");
displayArray(array3, length3);
performRadixSort(array4, length4);
printf("Sorted Array 4: ");
displayArray(array4, length4);
performRadixSort(array5, length5);
printf("Sorted Array 5: ");
displayArray(array5, length5);

return 0;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
}