0

In the following program,

    int * accepted_ids = (int *) malloc(sizeof(int)*N);
    double * accepted_scores = (double *)malloc(sizeof(double)*N);

    int * unaccepted_ids = (int *) malloc(sizeof(int)*N);
    double * unaccepted_scores = (double *) malloc(sizeof(double)*N);

these memory allocations are creating arrays of size N for each of them, even though the number of needed elements are much lower than that of N.

Since the program is using a random number generator, we can't tell beforehand how much memory we need for each of them.

How can I solve this dilemma?

(max. 5 points)
A single dimension array SCORES stores scores of N university candidates they gained at high school. Indexes of elements of the array are the IDs of these candidates. The university accepts applications from candidates with the average score greater or equal to 4.0.

Write a short program that will display:

• The list of accepted candidates with their ID number and their average score. • The list of unaccepted candidates with their ID numbers and their average score • The number of accepted and unaccepted candidates. • Results sorted in ascending order

The average scores should be calculated from a range <2,6> using random numbers generator. The total number of candidates should be passed to the program as command line parameter. Make your program sensible to the input of wrong parameters.

Using struct is not allowed.

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <errno.h>

// This function tests whether it is possible 
// to convert a string into integer or not.
//
// This function is needed to check the 
// input argument otherwise if you type 
//      C:\>myapp.exe abc
// your program will crash.
int is_integer(const char * s)
{
    char * endptr;
    int radix = 10;//decimal number system

    // try to convert s to integer
    strtol(s, &endptr, radix);

    errno = 0;
    // if this conditions are fullfilled,
    // that means, s can't be converted 
    // to an integer.
    if (endptr == s || *endptr != '\0')
    {
        // argument must be an integer value        
        return 0; // failure
    }
    if (errno == ERANGE)
    {
        // int argument out of range        
        return 0; // failure
    }

    return 1; //success
}

// This function is needed to convert
// a string to an integer value.
int string_to_integer(const char * s)
{
    char * endptr;
    int radix = 10;//decimal number system

    // convert s to integer
    return strtol(s, &endptr, radix);
}

// Generte a random number between M and N.
//
// This function is needed coz rand() can 
// generate only integer values.
double round_between_m_to_n(double M, double N)
{
    return M + (rand() / (RAND_MAX / (N - M)));
}

// This is Bubble sort algorithm
// This is implemented as a user-defined function, 
// coz, you have to use this twice.
// First for accepted scores, 
// then for unaccepted scores.
void sort(int * ids, double * scores, int count)
{
    for (int i = 0; i < count; i++)
    {
        for (int j = 0; j < i; j++)
        {
            if (scores[i] < scores[j])
            {
                // Swap scores
                double temp = scores[i];
                scores[i] = scores[j];
                scores[j] = temp;

                // Swap ids
                int temp2 = ids[i];
                ids[i] = ids[j];
                ids[j] = temp2;
            }
        }
    }
}

// This function is to print ids and scores
// as a table.
// This is implemented as a user-defined function, 
// coz, you have to use this twice.
// First for accepted scores, 
// then for unaccepted scores.
void print(int * ids, double * scores, int count)
{

    printf("id\tavg_score\n");
    printf("-------------------\n");
    for (int i = 0; i < count; i++)
    {
        printf("%i\t%.1f\n", ids[i], scores[i]);
    }
}

int main(int argc, char ** argv)
{
    // Program can proceed only if 
    // the # of arguments is exactly 2. 
    // The 1st arg is always app-name.
    if (argc != 2)
    {
        printf("insufficient argument\n");
        return EXIT_FAILURE;
    }

    int N = 0;
    int accepted_scores_count = 0;
    int unaccepted_scores_count = 0;
    double acceptance_threshhold = 4.0;

    if (!is_integer(argv[1]))
    {
        printf("incorrect argument type\n");
        return EXIT_FAILURE;
    }
    else
    {
        N = string_to_integer(argv[1]);
        printf("Total %d students\n", N);
    }

    // Pair of variables are needed to
    // keep track of student-ids.
    // Otherwise, you can't tell what id a 
    // student has when data are sorted.
    int * accepted_ids = (int *)malloc(sizeof(int)*N);
    double * accepted_scores = (double *)malloc(sizeof(double)*N);

    int * unaccepted_ids = (int *)malloc(sizeof(int)*N);
    double * unaccepted_scores = (double *)malloc(sizeof(double)*N);

    //Initialize random seed.
    //If you don't use this, rand() will generate
    //same values each time you run the program.
    srand(time(NULL));

    // Simultaneously generate scores, ids, and
    // store them is sepaerate arrays.
    for (int i = 0; i < N; i++)
    {
        int id = i;
        double score = round_between_m_to_n(2, 6);

        // if the score is greater than or
        // equal to 4.0...
        if (score >= acceptance_threshhold)
        {
            accepted_ids[accepted_scores_count] = i;
            accepted_scores[accepted_scores_count] = score;

            accepted_scores_count++;
        }
        // ... otherwise they are unaccepted.
        else
        {
            unaccepted_ids[unaccepted_scores_count] = i;
            unaccepted_scores[unaccepted_scores_count] = score;

            unaccepted_scores_count++;
        }
    }

    // sort accepted students
    sort(accepted_ids, accepted_scores, accepted_scores_count);
    // sort unaccpeted students
    sort(unaccepted_ids, unaccepted_scores, unaccepted_scores_count);

    // print accepted students
    printf("\naccepted  students\n");
    print(accepted_ids, accepted_scores, accepted_scores_count);

    // print unaccepted students
    printf("\nunaccepted  students\n");
    print(unaccepted_ids, unaccepted_scores, unaccepted_scores_count);

    printf("\nEnd of program.\n");

    free(accepted_ids);
    free(accepted_scores);
    free(unaccepted_ids);
    free(unaccepted_scores);


    return EXIT_SUCCESS;
}
user366312
  • 16,949
  • 65
  • 235
  • 452
  • 1
    *"these memory allocations are creating arrays of size N for each of them, even though the number of needed elements are much lower than that of N"* > You told the program to allocate that much. If you want less, find out how much you need, and then allocate that amount. –  Apr 25 '17 at 12:00
  • @InternetAussie, Yeah. That is the problem. Since the program is using a random number generator, we can't tell beforehand how much memory we need for each of them. – user366312 Apr 25 '17 at 12:01
  • 2
    [Don't cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – UnholySheep Apr 25 '17 at 12:02
  • @anonymous Ignore the advice of  UnholySheep. It is totally wrong. – Vlad from Moscow Apr 25 '17 at 12:03
  • Why do you need 4 arrays? Can't you create a `struct` with `id` and `score` and the program will calculate on the fly if the candidate got accepted? (Or just keep a bool in the `struct` after calculating once)? Then you'll have a single N-element array (or a linked list if `N` is unknown) – Rogus Apr 25 '17 at 12:06
  • 1
    @Rogus, using struct was prohibited. – user366312 Apr 25 '17 at 12:09
  • @VladFromMoscow it is absolutely correct. Even though it is not related to this question, it is a valid suggestion. – Ajay Brahmakshatriya Apr 25 '17 at 12:17
  • @AjayBrahmakshatriya It is totally wrong. It is a point of view of low-qualified programmers. I already down-voted the answer. For example consider the code p = malloc( sizeof( *p ) ); Can you say what type of memory is allocated? Can you say whether it is a valid code? – Vlad from Moscow Apr 25 '17 at 12:20
  • 1
    @VladfromMoscow-- you are wrong about this. There is room for disagreement whether it is better to cast or not to cast the result of `malloc()`, but it is not "totally wrong" to avoid the cast, and is in fact a common idiom used by experienced (and well-qualified) programmers. – ad absurdum Apr 25 '17 at 12:25
  • @DavidBowling You have not answered my question. One more can you say what type of object is allocated in this statement p = malloc( sizeof *p ); and whether this code is valid? I can answer instead of you. You can say NOTHING. This statement is black box that only confuses the reader of the code. – Vlad from Moscow Apr 25 '17 at 12:33
  • 1
    @VladfromMoscow-- this is a contrived example. 1) I would typically allocate on the same line as the declaration, as OP code does. 2) I am not religious about this, but personally avoid the cast unless it makes sense for some reason; it _may_ make code more legible in your example. 3) I am in general against statements like "don't cast the result of `malloc()`" as well as statements like "it is totally wrong to fail to cast the result of `malloc()`." More nuance, please. – ad absurdum Apr 25 '17 at 12:52
  • 1
    Note: `is_integer(const char * s)` fails to report overflow errors should `long` have a wider range than `int` and the converted value exceeds `int`. – chux - Reinstate Monica Apr 25 '17 at 15:11

2 Answers2

1

If you want to end up allocating less memory, then use realloc. Start by reallocing 0 items, then each time you need to allocate some more, use realloc to allocate more memory. As realloc will 'copy' the existing data, you will only end up with the memory you actually require. Bear in mind that realloc is not a great function, and it's use should be done with care, as it is very easy to get wrong (make sure you check return values, and keep track of the previous allocations before overwriting pointers).

Neil
  • 11,059
  • 3
  • 31
  • 56
  • Infact the old pointer shouldn't be retained since it is deallocated by realloc. Using the old pointer would lead to a use after free. – Ajay Brahmakshatriya Apr 25 '17 at 12:15
  • @AjayBrahmakshatriya The old pointer needs to be retained in case `realloc` fails. If `realloc` fails, then the old pointer is still valid. Discarding it would leak memory. You can only discard the old pointer if `realloc` succeeds. –  Apr 25 '17 at 12:17
  • @InternetAusie. I agree with that, but how the last statement is written it hints that previous pointer should be retained elsewhere. If it is substantially clear to others, okay. – Ajay Brahmakshatriya Apr 25 '17 at 12:20
  • realloc is a horrible mishmash of functionality, it's very easy to misuse and leave dangling pointers, hence my warning. – Neil Apr 25 '17 at 13:06
  • 1
    One can describe how to used `realloc()`. Even better, post a good code use. – chux - Reinstate Monica Apr 25 '17 at 15:00
1

Because you know the number of students to generated data for, you can use arrays of data for all students:

    int * all_ids = (int *)malloc(sizeof(int)*N);
    double * all_scores = (double *)malloc(sizeof(int)*N);

Then generate data as normal, keeping counts, but assign the data into the all_* arrays:

    for (int i = 0; i < N; i++)
    {
        int id = i;
        double score = round_between_m_to_n(2, 6);

        all_ids[i] = id;
        all_scores[i] = score;

        // if the score is greater than or
        // equal to 4.0...
        if (score >= acceptance_threshhold)
        {
            accepted_scores_count++;
        }
        // ... otherwise they are unaccepted.
        else
        {
            unaccepted_scores_count++;
        }
    }

Because you know the thresholds for distinguishing accepted students, you can split these later.

Now you have all the data, and the numbers of students who were accepted and not accepted. Using this information, you can allocated the arrays for the accepted and unaccepted students:

    int * accepted_ids = (int *)malloc(sizeof(int) * accepted_scores_count);
    double * accepted_scores = (double *)malloc(sizeof(double) * accepted_scores_count);

    int * unaccepted_ids = (int *)malloc(sizeof(int) * unaccepted_scores_count);
    double * unaccepted_scores = (double *)malloc(sizeof(double) * unaccepted_scores_count);

Sort the data into the accepted and unaccepted arrays as you do originally, with the for loop (minus the data generation, since that is done):

    for (int i = 0, j = 0; (i+j) < N;)
    {
        int id = all_ids[i+j];
        double score = all_scores[i+j];

        // if the score is greater than or
        // equal to 4.0...
        if (score >= acceptance_threshhold)
        {
            accepted_ids[i] = id;
            accepted_scores[i] = score;

            i++;
        }
        // ... otherwise they are unaccepted.
        else
        {
            unaccepted_ids[j] = id;
            unaccepted_scores[j] = score;

            j++;
        }
    }

After that, you continue as normal sorting and printing your data. You must remember to free the all_* arrays too.