0

I'm quite new to C, I'm learnig it cause of my school and i can say I'm starting to understand why most laguages are based on this one.

I'm trying to make a programm that creates an int array of 100 random numbers saves it to a file and then eliminates duplicate numbers and sorts it in ascending order.

My solustion brings out in to the same file the desired effect but it feels kind of too complicated for what it is suppose to do.

I'm 99% certain that there is a more delicate or simple way to do this.

I was gonna use 3 function:

  1. For saving the file
  2. For eliminating duplicates and,
  3. For sorting

In the end I did 2 cause I encoutered a few problems in my solution.

e.x.:One of the problems is that after deleting duplicates I was left with a bunch of places in my array that were filled with values I didn't whant, like the sort of values you get in a variable that you haven't initialize. So I used counter variables ending up using global vars which they weren't my first choice.

I'm pretty sure you can find a few crazy things in there.

I'm open to any suggestion if there is any and I would like to thank you all in advance for trying to help me find the end of my own mess.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int cnt;

void *save(int *pin)
{
    FILE *dat;
    int i=0;
    dat = fopen("data.txt","w");
    while(pin[i])
    {
    if(i==cnt){break;}
    fprintf(dat,"\n%d",pin[i]);
    i++;
    }
    fclose(dat);
    return 0;
}

void del_sort()
{
    FILE *dat;
    int pin[100];
    int i=0,j,k,sw=0,temp;
    dat=fopen("data.txt","r");
    while(!feof(dat))
    {
        fscanf(dat,"%d",&pin[i]);
        i++;
    }
    fclose(dat);

    //Eliminate duplicate numbers
    for(i=0;i<cnt;i++)
    {
        sw++;
        if (pin[i]>100)
            {
                sw--;
                break;
            }
        for(j=i+1;j<cnt;j++)
        {
            if (pin[i]==pin[j])
        {
            for(k=j;k<cnt;k++)
            {
                pin[k]=pin[k+1];
            }
        }
    }
}

int pin2[sw];
for(i=0;i<sw;i++)
{
    pin2[i]=pin[i];
}
//Sort number in Ascending order
for(i=0;i<sw-1;i++)
{
    for(j=0;j<sw-i-1;j++)
    {
        if(pin2[j]>pin2[j+1])
        {
            temp=pin2[j];
            pin2[j]=pin2[j+1];
            pin2[j+1]=temp;
        }
    }
}
printf("\n\n\n");
for(i=0;i<sw;i++)
{
    printf("%d\t",pin2[i]);
}
cnt = sw;
save(pin2);
}

int main()
{
srand(time(0));
int pin[100];
int i;

cnt=0;
for(i=0;i<100;i++)
{
    pin[i]=rand()%100+1;
    printf("%d\t",pin[i]);
    cnt++;
}
printf("\n");

save(pin);
del_sort();

return 0;
}
5
  • 3
    Read Why is “while ( !feof (file) )” always wrong?. You can use like while(fscanf(dat,"%d",&pin[i]) == 1) { } instead of feof(). Commented Oct 27, 2018 at 17:44
  • That is a nice article there and thanks for showing me this but still was way far from the actual problem of this code. I've seen the problem of foef() in a while loop before but in this it was handled by simply adding the \n before the %d and so the output of fscanf() is good. Commented Oct 27, 2018 at 19:36
  • If you sort first, you can then easily remove duplicate items by walking through the array in order and comparing the previous and current elements. Commented Oct 27, 2018 at 23:55
  • "I'm learnig it cause of my school" - then you go to a good school. Do not look on it as a burden. If you want to learn to "program", then you need to learn C (or assembly). It will make you a better programmer regardless what language you end up using. Anyone can learn a language, few learn to "program". Commented Oct 28, 2018 at 1:25
  • It's not a burden mate I'm just saying. My choice was python for many reasons not meaning that I don't like C. And for the record is not a school for children. It's a tech school Commented Oct 28, 2018 at 8:31

1 Answer 1

2
void *save(int *pin)
{
   while(pin[i])
   {
       if(i==cnt){break;}
       ...
   }
   return 0;
}

Testing the value pin[i] is not recommended. You are passing an array of unknown size, the program doesn't know where the array ends, you may end up with buffer overrun.

You do check i==cnt, so that actually saves the day and buffer overrun is prevented. But it's better to rewrite the loop based on cnt. Also change void* to void

The main issue in the code is loop for eliminating duplicates. Just check if duplicate value exists, if it does, then skip to next iteration.

int cnt;
const char* filename = "data.txt";

void save(int *pin)
{
    FILE *dat = fopen(filename, "w");
    for(int i = 0; i < cnt; i++)
        fprintf(dat, "%d\n", pin[i]);
    fclose(dat);
}

void del_sort()
{
    FILE *dat;
    int pin[100];
    int i = 0, j, k, sw, temp;
    dat = fopen(filename, "r");
    while(fscanf(dat, "%d\n", &pin[i]) == 1)
    {
        i++;
        if(i == cnt)
            break;
    }
    fclose(dat);

    int pin2[100];
    sw = 0;
    //Eliminate duplicate numbers
    for(i = 0; i < cnt; i++)
    {
        int duplicate = 0;
        for(j = i + 1; j < cnt; j++)
        {
            if(pin[i] == pin[j])
            {
                duplicate = 1;
                break;
            }
        }
        if(duplicate)
            continue;
        pin2[sw] = pin[i];
        sw++;
    }

    //Sort number in Ascending order
    for(i = 0; i < sw - 1; i++)
    {
        for(j = 0; j < sw - i - 1; j++)
        {
            if(pin2[j] > pin2[j + 1])
            {
                temp = pin2[j];
                pin2[j] = pin2[j + 1];
                pin2[j + 1] = temp;
            }
        }
    }

    printf("sort\n");
    for(i = 0; i < sw; i++)
        printf("%d, ", pin2[i]);

    cnt = sw;
    save(pin2);
}

int main(void)
{
    srand((unsigned int)time(0));
    int pin[100] = { 0 };
    int i;

    cnt = 100;
    for(i = 0; i < cnt; i++)
    {
        pin[i] = rand() % 100 + 1;
        printf("%d, ", pin[i]);
    }
    printf("\n");

    save(pin);
    del_sort();

    return 0;
}

To remove duplicates in pin without allocating pin2 use the following (similar to your own method, except the calculation of sw)

sw = cnt;
for(i = 0; i < sw - 1; i++)
{
    for(j = i + 1; j < sw; j++)
    {
        if(pin[i] == pin[j])
        {
            //duplicate found
            for(k = j; k < sw - 1; k++)
            {
                pin[k] = pin[k + 1];
            }
            sw--;

            //decrement so we can check for consecutive duplicates
            i--;
        }
    }
}
Sign up to request clarification or add additional context in comments.

9 Comments

So you basically say to avoid deleting from pin an just put all unique values in pin2. Not a bad idea at all! Also about the while you're right. cnt var came afterwards while I was trying to debug it
Also a question I have is that except passing the unique values in pin2 is it possible to manipulate pin deleting values without some kind of malloc function? Like e.x. putting a null in the empty spot somehow.
@BarmakShemirani hmm.. careful there, if I understand your comment "NULL is just zero", technically, that's not quite true. The value for the ASCII nul-character is just zero, but the value for NULL is a pointer (generally zero cast to void*, e.g. (void*)0) So technically NULL is a pointer and 0 is an integer.
@DavidC.Rankin You are right, I was trying to explain something in a few words and failed. I am going to try again!
"Like e.x. putting a null in the empty spot" @gmonster1st, pin[index] expects an integer value, therefore pin[index] = NULL; is wrong because NULL is a pointer, not integer (although it may still compile with a warning). I think your idea is to remove an element in the array, like in a high level language. It is not possible to do that automatically in C. See edit for alternate method to delete duplicates in pin
|

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.