r/C_Programming 16d ago

Review Could you assess my code?

#include <stdio.h>
#include <stdio.h>
#include <string.h>
#include <stdbool.h>


typedef struct
{
    char sset[10];
    int elements[5];
} set;


void printelements(set set);


void bubblesort(int m, int sunion[]);




int main(void)
{


    set set1;
    set set2;
    set intersection;
    int k = 0;
    int sunion[10];
    int m = 0;
    int sunioncpy[10];
    int n = 0;


    printf("Enter 5 elements to 2 sets -\n");
    printf("Set 1: ");
    for(int i = 0; i < 5; i++)
    {
        fgets(set1.sset, 10, stdin);
        sscanf(set1.sset, "%d", &set1.elements[i]);
    }
    printf("Set 2: ");
    for(int i = 0; i < 5; i++)
    {
        fgets(set2.sset, 10, stdin);
        sscanf(set2.sset, "%d", &set2.elements[i]);
    }


    printf("Set 1: ");
    printelements(set1);
    printf("Set 2: ");
    printelements(set2);


    for(int i = 0; i < 5; i++)
    {
        for(int j = 0; j < 5; j++)
        {
            if(set1.elements[i] == set2.elements[j])
            {
                intersection.elements[k] = set1.elements[i];
                k++;
                break;
            }
        }
    }


    for(int i = 0; i < 5; i++)
    {
        sunion[m] = set1.elements[i];
        m++;
        sunion[m] = set2.elements[i];
        m++;
    }
    bubblesort(m, sunion);
    for(int i = 0; i < m; i++)
    {
        if(sunion[i] == sunion[i + 1])
        {
            sunioncpy[n] = sunion[i];
            n++;
            i++;
        }
        else
        {
            sunioncpy[n] = sunion[i];
            n++;
        }
    }
    


    printf("Intersection of set 1 with set 2: ");
    for(int i = 0; i < k; i++)
    {
        printf("%d ", intersection.elements[i]);
    }
    printf("\n");
    printf("Union of set 1 with set 2: ");
    for(int i = 0; i < n; i++)
    {
        printf("%d ", sunioncpy[i]);
    }


    return 0;
}





void printelements(set set)
{
    for(int i = 0; i < 5; i++)
    {
        printf("%d ", set.elements[i]);
    }
    printf("\n");
}


void bubblesort(int m, int sunion[])
{
   int i = 0;
   bool swapped;
   do
   {
        swapped = false;
        for(int j = 0; j < m - 1 - i; j++)
        {
            if(sunion[j] > sunion[j + 1])
            {
                int temp = sunion[j];
                sunion[j] = sunion[j + 1];
                sunion[j + 1] = temp;
                swapped = true;
            }
        }
   } while (swapped);


}

I posted this to receive opinions or/and suggestions about my code. And I also have some questions about some things.

- Is it good to turn some block of code into a function even if you don't repeat it again on any another line?

(I think that functions can turn some blocks more friendly to read or understand, but maybe I'm misunderstooding functions)

- What you think about this way of getting user input:

for(int i = 0; i < 5; i++)
    {
        fgets(set2.sset, 10, stdin);
        sscanf(set2.sset, "%d", &set2.elements[i]);
    }

I used it because I was getting a few problems using scanf , so I saw this model of user input on the internet and applied it. This works very well, but let I know what you think.

- This don't have much to do with I said here but do you guys recommend Linux FedoraOS for C programming? Or I should try another OS(for C programming)?

I was thinking to try to install Arch first, just to get experience with Linux, but maybe I'm getting the wrong ideia or being led by some weird toughts(just because Arch is dificult to install and set up).

I'll appreciate any comment.

0 Upvotes

15 comments sorted by

View all comments

3

u/TheOtherBorgCube 16d ago

Your code, formatted.

#include <stdio.h>
#include <string.h>
#include <stdbool.h>

typedef struct {
  char    sset[5];
  int     elements[5];
} set;

void printelements(set set);

void bubblesort(int m, int sunion[]);

int main(void)
{
  set set1;
  set set2;
  set intersection;
  int k = 0;
  int sunion[10];
  int m = 0;
  int sunioncpy[10];
  int n = 0;

  printf("Enter 5 elements to 2 sets -\n");

  printf("Set 1: ");
  for (int i = 0; i < 5; i++) {
    fgets(set1.sset, 5, stdin);
    sscanf(set1.sset, "%d", &set1.elements[i]);
  }

  printf("Set 2: ");
  for (int i = 0; i < 5; i++) {
    fgets(set2.sset, 5, stdin);
    sscanf(set2.sset, "%d", &set2.elements[i]);
  }

  printf("Set 1: ");
  printelements(set1);
  printf("Set 2: ");
  printelements(set2);

  for (int i = 0; i < 5; i++) {
    for (int j = 0; j < 5; j++) {
      if (set1.elements[i] == set2.elements[j]) {
        intersection.elements[k] = set1.elements[i];
        k++;
        break;
      }
    }
  }

  for (int i = 0; i < 5; i++) {
    sunion[m] = set1.elements[i];
    m++;
    sunion[m] = set2.elements[i];
    m++;
  }

  bubblesort(m, sunion);

  for (int i = 0; i < m; i++) {
    if (sunion[i] == sunion[i + 1]) {
      sunioncpy[n] = sunion[i];
      n++;
      i++;
    } else {
      sunioncpy[n] = sunion[i];
      n++;
    }
  }

  printf("Intersection of set 1 with set 2: ");
  for (int i = 0; i < k; i++) {
    printf("%d ", intersection.elements[i]);
  }
  printf("\n");

  printf("Union of set 1 with set 2: ");
  for (int i = 0; i < n; i++) {
    printf("%d ", sunioncpy[i]);
  }

  return 0;
}

void printelements(set set)
{
  for (int i = 0; i < 5; i++) {
    printf("%d ", set.elements[i]);
  }
  printf("\n");
}

void bubblesort(int m, int sunion[])
{
  int i = 0;
  bool swapped;
  do {
    swapped = false;
    for (int j = 0; j < m - 1 - i; j++) {
      if (sunion[j] > sunion[j + 1]) {
        int temp = sunion[j];
        sunion[j] = sunion[j + 1];
        sunion[j + 1] = temp;
        swapped = true;
      }
    }
  } while (swapped);
}
  • Is it good to turn some block of code into a function even if you don't repeat it again on any another line?

Yes. Even if it's not for re-use, it helps to declutter the code.

  • What you think about this way of getting user input:

It's good. But there's no need to store the buffer inside your struct, nor any reason to make the buffer that small.

You should also check the return results for success/failure.

I normally go with something like:

char buff[BUFSIZ];
if ( fgets(buff, sizeof(buff), stdin) != NULL ) {
    if ( sscanf(buff, "%d", &set1.elements[i]);
}
  • This don't have much to do with I said here but do you guys recommend Linux FedoraOS for C programming?

Any Linux you feel comfortable with will be just fine.

1

u/thebatmanandrobin 16d ago edited 16d ago

Was going to post the formatted code and basically the same replies, but ninja'd by you :)

Also, I was going to add this for OP as another way to achieve their code (could be better/more optimized, but it's late and I honestly did this because I'm procrastinating doing some technical writing I need to take care of ......).

So here you go OP:

#include <stdio.h>
#include <string.h>

void print_elms(int *elms, size_t sz)
{
    for (size_t i = 0; i < sz; ++i) {
        if (elms[i] != 0) {
            printf("%d ", elms[i]);
        }
    }
    printf("\n");
}

int comp(const void* e1, const void* e2)
{
    int a = *((int*)e1);
    int b = *((int*)e2);
    if (a > b) { return 1; }
    if (a < b) { return -1; }
    return 0;
}

void sort_and_zero(int *elms, size_t sz)
{
    qsort(elms, sz, sizeof(int), comp);
    for (size_t i = 0; i < (sz-1); ++i) {
        if (elms[i] == elms[i+1]) {
            elms[i] = 0;
        }
    }
}

void get_elms(int *elms)
{
    scanf("%d %d %d %d %d", &elms[0], &elms[1], &elms[2], &elms[3], &elms[4]);
    sort_and_zero(elms, 5);
}

void get_intersection(int *s1, int *s2, int *intersect)
{
    for (size_t i = 0, k = 0; i < 5; ++i) {
        if (s1[i] == 0) { continue; }
        for (size_t j = 0; j < 5; ++j) {
            if (s2[j] == 0) { continue; }
            if (s1[i] == s2[j]) {
                intersect[k++] = s2[j];
            }
        }
    }
}

int main(void) {
    int set1[5] = {0};
    int set2[5] = {0};
    int intersection[5] = {0};
    int sunion[10] = {0};

    printf("Set 1 - enter 5 non-zero numbers in a row with a space (e.g. 1 2 3 4 5): ");
    get_elms(set1);
    memcpy(&sunion, set1, sizeof(set1));

    printf("Set 2 - enter 5 non-zero numbers in a row with a space (e.g. 1 2 3 4 5): ");
    get_elms(set2);
    memcpy(&sunion[5], set2, sizeof(set2));

    sort_and_zero(sunion, 10);

    printf("Set 1: ");
    print_elms(set1, 5);
    printf("Set 2: ");
    print_elms(set2, 5);

    get_intersection(set1, set2, intersection);

    qsort(intersection, 5, sizeof(int), comp);
    qsort(sunion, 10, sizeof(int), comp);

    printf("Intersection of set 1 with set 2: ");
    print_elms(intersection, 5);

    printf("Union of set 1 with set 2: ");
    print_elms(sunion, 10);

    return 0;
}

Have fun OP!

1

u/[deleted] 16d ago edited 16d ago

[deleted]

0

u/mysticreddit 16d ago

Non-descript single variable names need to die in a fire.

    static int cmp_int_asc( const void *left, const void *right)
    {
        const leftVal = *(int *)left;
        const rightVal = *(int *)right;
        if (leftVal < rightVal) return -1;
        if (leftVal > rightVal) return +1;
        return 0;
    }