I'm trying to write a program which updates the smallest number of an array by itself iteratively until all the numbers in the array are the same (an algorithm for finding the lcm). My program seems to end up in an endless loop. My guess is that it is caused by are_same() returning always zero caused by something in get_min_index() and/or divisors[min_index] += divisors[min_index]. I'm wondering if this has to do with some C-language peculiarity I'm not aware of. Could some experienced C programmer help me out with this?
#include <stdio.h>
int are_same(int array[], int length) {
for (int i = 1; i < length; i++) {
if (array[0] != array[i]) {
return 0;
}
}
return 1;
}
int get_min_index(int array[], int length) {
int min_value = array[0];
int min_index = 0;
for (int i = 1; i < length; i++) {
if (array[i] < min_value) {
min_value = array[i];
min_index = i;
}
}
return (min_index);
}
int main(void) {
int divisors[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
int length = sizeof(divisors) / sizeof(divisors[0]);
int min_index;
do {
min_index = get_min_index(divisors, length);
divisors[min_index] += divisors[min_index];
} while (are_same(divisors, length) == 0);
printf("lcm=%d", divisors[0]);
return(0);
}
Your are_same implementation is fine and does the job.
The algorithm, to find the smallest and to that add its corresponding divisor, then search for the smallest and do the same repeatedly, has a bug since you double the value instead of adding the divisor to it.
Even properly implemented, it will likely take a long time to finish since you search through the whole array in order to just update one value once. It would be faster if you instead could add the product of its divisor muliplied by x so that it becomes at least as large as the largest value in the array.
A slight modification would then be to instead
array with a multiple of the corresponding divisor so that the value becomes at least as large as the largest value.I don't care about the index of the largest in this example, so this simply returns the largest value:
int largest_value(int arr[], size_t length) {
int l = arr[0];
for(size_t i = 1; i < length; ++i) {
if(arr[i] > l) l = arr[i];
}
return l;
}
A calculate_lcm function would need to copy the incoming array (to fix the bug in your current implementation) and implement the algorithm described above:
int calculate_lcm(const int array[], size_t length) {
// copy array in to a "working array", wa:
int* wa = malloc(length * sizeof *wa);
if(!wa) exit(1);
memcpy(wa, array, length * sizeof *wa);
while(!are_same(wa, length)) {
// find the largest value in wa:
int large = largest_value(wa, length);
for(size_t i = 0; i < length; ++i) {
// make wa[i] at least as big as `large`
wa[i] += ((large - wa[i] + array[i] - 1) / array[i]) * array[i];
}
}
int lcm = wa[0];
free(wa);
return lcm;
}
You could also skip calling all_same and just check if the largest value has changed since the previous iteration. If it has not, all numbers are the same:
int calculate_lcm(const int array[], size_t length) {
int* wa = malloc(length * sizeof *wa);
if (!wa) exit(1);
memcpy(wa, array, length * sizeof *wa);
for(int largest = array[0];;) {
int newlargest = largest_value(wa, length);
if(newlargest == largest) break;
largest = newlargest;
for (size_t i = 0; i < length; ++i) {
wa[i] += ((largest - wa[i] + array[i] - 1) / array[i]) * array[i];
}
}
int lcm = wa[0];
free(wa);
return lcm;
}