0
\$\begingroup\$

I had to make a program that have an array of numbers, and then I need to make a function that get the arr[0] and the length of the array, and then it will print all the numbers without the duplicate ones.

I made this program and it worked well but I feel like I used too many variables for this program. (I started to learn in the last few weeks so in my opinion it does not look very good)

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int i = 1;
    int duplicate_num, check_num, count;
  
    printf(" %d", *arr); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    arr++;
    while (i < n) {
        if (*arr != duplicate_num) {
            printf(" %d", *arr);
            check_num = *arr;
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            while (i < n) {
                if (*arr == check_num) {
                    *arr = duplicate_num;
                }
                arr++;
                i++;
                count++;
            }
            i = i - count;
            arr = arr - count;
            count = 0;
        }
        arr++;
        i++;
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(&arr[0], N);
    return 0;
}

output for this program: 4 6 9 8 1

I'll be happy to see any good methods to make this program less messy.

\$\endgroup\$
1
  • \$\begingroup\$ The title of the question should be about what the code does rather than your concerns about the code, please read How do I ask a good question?. \$\endgroup\$
    – pacmaninbw
    Jun 17, 2022 at 14:31

3 Answers 3

1
\$\begingroup\$

The method you used to solve this problem is correct and the implementation is correct. The changes I suggest below are about making the code correct and making the code easier to read so that someone reading the code (including you in the future) will understand how this problem is solved.

The first thing I notice is that you are modifying both the array index i and the address of the first element arr in order to track your current location in the array. So, you have to modify i and arr at the same time. This is unnecessary. You only need to modify i and then use arr[i] to get the number at that index. So, let's replace every instance of *arr with arr[i] and deleted all of the lines that modify arr.

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int i = 1;
    int duplicate_num, check_num, count;
  
    printf(" %d", arr[0]); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    while (i < n) {
        if (arr[i] != duplicate_num) {
            printf(" %d", arr[i]);
            check_num = arr[i];
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            while (i < n) {
                if (arr[i] == check_num) {
                    arr[i] = duplicate_num;
                }
                i++;
                count++;
            }
            i = i - count;
            count = 0;
        }
        i++;
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, N);
    return 0;
}

Next, the variables duplicate_num and check_num are not initialized with values. In fact, duplicate_num is never assigned a value, so using it in the if (arr[i] != duplicate_num) and arr[i] = duplicate_num is undefined behavior. See the note at the very bottom of this answer for how the compiler can help you find these problems.

I see that duplicate_num is used as a marker for array values that have been seen before, therefore it needs to be a value that cannot appear in the array. Since you print arr[0] outside the loop, that is actually a good value. Making this change:

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int i = 1;
    int duplicate_num = arr[0], check_num, count;
  
    printf(" %d", arr[0]); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    while (i < n) {
        if (arr[i] != duplicate_num) {
            printf(" %d", arr[i]);
            check_num = arr[i];
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            while (i < n) {
                if (arr[i] == check_num) {
                    arr[i] = duplicate_num;
                }
                i++;
                count++;
            }
            i = i - count;
            count = 0;
        }
        i++;
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, N);
    return 0;
}

The variable check_num is always assigned a value before it is used, so it is less of a problem. Still, you always need to make sure that you never use a variable until after it has been assigned a value.

Now, the variable count seems unnecessary. You are using it to get back to the initial value of i after marking all the duplicate numbers in the rest of the list. Instead, let's use a second index j to loop through the rest of the list so we don't need to reset the value of i afterwards.

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int i = 1, j = 0;
    int duplicate_num = arr[0], check_num;
  
    printf(" %d", arr[0]); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    while (i < n) {
        if (arr[i] != duplicate_num) {
            printf(" %d", arr[i]);
            check_num = arr[i];
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            for (j = i; j < n; j++) {
                if (arr[j] == check_num) {
                    arr[j] = duplicate_num;
                }
            }
        }
        i++;
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, N);
    return 0;
}

Now we see that i is only modified at the end of the loop block. Let's make this a standard for-loop.

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int i = 0, j = 0;
    int duplicate_num = arr[0], check_num;
  
    printf(" %d", arr[0]); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    for (i = 1; i < n; ++i) {
        if (arr[i] != duplicate_num) {
            printf(" %d", arr[i]);
            check_num = arr[i];
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            for (j = i; j < n; j++) {
                if (arr[j] == check_num) {
                    arr[j] = duplicate_num;
                }
            }
        }
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, N);
    return 0;
}

Finally, if you are using a modern version of C, you can declare your variables where you need them, rather than at the top of the function. Let's move the variable declarations to where the variable is first needed.

#include <stdio.h>

#define N 10

void print_set(int *arr, int n) {
    int duplicate_num = arr[0];
  
    printf(" %d", arr[0]); //printing the first number (arr[0]).
    
    //starting to check the other number. if they new I'll print them. (start from arr[1]).
    for (int i = 1; i < n; ++i) {
        if (arr[i] != duplicate_num) {
            printf(" %d", arr[i]);
            int check_num = arr[i];
            // becouse I found new number, I change it no be equal to the first duplicate_num. (if there are more like him, I change them too).
            for (int j = i; j < n; j++) {
                if (arr[j] == check_num) {
                    arr[j] = duplicate_num;
                }
            }
        }
    }
}



int main() {
    int arr[N] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, N);
    return 0;
}

Note: Learn about compiling your programs with warnings turned on in order to have the compiler display messsages about these and other problems.

For gcc and clang: -Wall ("Warn on all problems") and -Wextra ("Warn on an extra set of problems").

For Microsoft Visual Studio: Project -> Properties -> C/C++ -> Warning Level = Level3 (/W3)

\$\endgroup\$
0
0
\$\begingroup\$

Suggest sorting the array then only a single pass through the array, comparing the current array value with the next array value will tell you if the value is a duplicate or not

\$\endgroup\$
1
  • 1
    \$\begingroup\$ This prints the values in sorted order, not the original order. Unknown what order is required. \$\endgroup\$ Jun 21, 2022 at 19:59
0
\$\begingroup\$

You can guarantee \$\Theta(n)\$ running time if you store already seen ints in a hash table:

#include "unordered_set.h"
#include <stdio.h>
#include <stdlib.h>

size_t hash_function(void* p) {
    return (size_t) p;
}

bool equals_function(void* a, void* b) {
    return a == b;
}

void print_set(int* arr, size_t n) {
    unordered_set* set = unordered_set_t_alloc(10,
                                               1.0f, 
                                               hash_function,
                                               equals_function);

    for (size_t i = 0; i < n; ++i) {
        if (!unordered_set_t_contains(set, arr[i])) {
            unordered_set_t_add(set, arr[i]);
            printf("%d ", arr[i]);
        }
    }
}

int main() {
    int arr[] = {4, 6, 9, 8, 6, 9, 6, 1, 6, 6};
    print_set(arr, sizeof(arr) / sizeof(arr[0]));
    return 0;
}

(Above, I used this project.)

\$\endgroup\$
1
  • \$\begingroup\$ "You can guarantee Θ(n)" --> unordered_set_t_add() worse case in not Θ(n) unless you are going for Θ(range_of_int) \$\endgroup\$ Jun 21, 2022 at 20:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.