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
33 changes: 33 additions & 0 deletions math/factorial_recursion.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#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?


#define MAX_LIMIT 200

unsigned long long factResults[MAX_LIMIT] = {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
unsigned long long factResults[MAX_LIMIT] = {0};
uint64_t factResults[MAX_LIMIT] = {0};


unsigned long long computeFactorial(int sequenceNumber) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

using lengthy data types is not recommended
use the _t types from stdint.h

#include<stdint.h>

uint64_t computeFactorial(...)
Suggested change
unsigned long long computeFactorial(int sequenceNumber) {
uint64_t computeFactorial(int 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];
}

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) {

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
}
}