2

Task:

From the keyboard, enter a sequence of records with information about the capitals of European states: , , , . A public holiday date is specified as a character string in the form Day of the Month, for example August 24. Then rearrange the data in calendar order according to the dates of the main public holiday.

Attempt:

The code below is a bit complex and long, I would like to either split it into functions or replace it altogether with something simpler

const char *months[MONTHS_NUM] = {
    "jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct",
    "nov", "dec"
};

typedef struct capital {
    char name[20];
    long int population;
    char holiday[60];
    char date[15];
} CAPITAL;

void
Sort_by_Date(CAPITAL arr[], int nc)
{
    int i, j;
    CAPITAL temp;
    int day1, day2;
    char month1[4], month2[4];
    int month1_idx, month2_idx;

    for (i = 0; i < nc - 1; i++) {
        for (j = i + 1; j < nc; j++) {
            sscanf(arr[i].date, "%d %3s", &day1, month1);
            sscanf(arr[j].date, "%d %3s", &day2, month2);
            for (month1_idx = 0; month1_idx < MONTHS_NUM; month1_idx++) {
                if (strcmp(months[month1_idx], month1) == 0)
                    break;
            }
            for (month2_idx = 0; month2_idx < MONTHS_NUM; month2_idx++) {
                if (strcmp(months[month2_idx], month2) == 0)
                    break;
            }
            if (month1_idx != month2_idx)
                return month1_idx < month2_idx;
            else if (day1 != day2)
                return day1 < day2;
            if (month1_idx > month2_idx) {
                temp = arr[i];
                arr[i] = arr[j];
                arr[j] = temp;
            }
            else if (month1_idx == month2_idx) {
                if (day1 > day1) {
                    temp = arr[i];
                    arr[i] = arr[j];
                    arr[j] = temp;
                }
            }
        }
    }
}
Craig Estey
  • 30,627
  • 4
  • 24
  • 48
mary
  • 67
  • 5
  • 3
    Using standard `qsort()` would be more efficient, easier, and cleaner. You can write its comparison function to sort by the string that contains the date. I'd write an answer, but there are already too many SO questions about this. The man page for `qsort()` also contains an example that you'd find helpful. And if you're not on UNIX-based system, a google search would suffice. – Harith Feb 04 '23 at 18:16
  • 1
    Start with an array of `int`s. Then move onto `struct`s. See https://stackoverflow.com/q/57098974/20017547 and https://stackoverflow.com/q/59890582/20017547 – Harith Feb 04 '23 at 18:30
  • 2
    mary, BTW: good use of `3` in `%3s` for a `char month1[4]`. – chux - Reinstate Monica Feb 04 '23 at 20:09
  • 1
    See https://stackoverflow.com/q/75242335/17592432 for a tiny, fast hash function that directly returns the month ordinal when passed a string containing the name of a month. Using this would eliminate the "groping search" for the conversion. – Fe2O3 Feb 04 '23 at 21:30

2 Answers2

2

A few issues ...

  1. Sort_by_Date should sort the entire array.
  2. It loops on all dates, but it does a return on a mismatch.
  3. The latter is what a sort comparison function should do.
  4. The function also tries to swap out of order elements. That is what a sort function would do.
  5. We need to split up the function into a sort function (similar to qsort) and a comparison function (similar/compatible with the last arg of qsort).
  6. The compare function has to compare two keys: The month index and then the day value.
  7. There is replicated code for decoding a date into a month index. This should be a separate function.

Here is a refactored version. It is annotated.

It has two versions of the sort function:

  1. A wrapper around qsort
  2. Original sort function cleaned up.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MONTHS_NUM      12

const char *months[MONTHS_NUM] = {
    "jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct",
    "nov", "dec"
};

typedef struct capital {
    char name[20];
    long int population;
    char holiday[60];
    char date[15];
} CAPITAL;

// date_sort_key -- convert date into single/combined sort key
// RETURNS: mon|day
int
date_sort_key(const char *date)
{
    int monidx = -1;
    int day;
    char mon[10];

    // split date into day/month
    sscanf(date,"%d %3s",&day,mon);

    // convert month string into an index of (0-11)
    for (int idx = 0;  idx < MONTHS_NUM;  ++idx) {
        if (strcmp(months[idx],mon) == 0) {
            monidx = idx;
            break;
        }
    }

    // fail if month string is invalid
    if (monidx < 0) {
        fprintf(stderr,"date_sort_key: invalid month -- '%s'\n",mon);
        exit(1);
    }

    // get combined key of: mon|day
    monidx <<= 8;
    monidx |= day;

    return monidx;
}

// compare_dates -- qsort compatible compare function
int
compare_dates(const void *vlhs,const void *vrhs)
{
    const CAPITAL *lhs = vlhs;
    int lhsidx = date_sort_key(lhs->date);

    const CAPITAL *rhs = vrhs;
    int rhsidx = date_sort_key(rhs->date);

    // compare the two date keys
    int cmp = rhsidx - lhsidx;

    return cmp;
}

void
Sort_by_Date_qsort(CAPITAL arr[], int nc)
{

    qsort(arr,nc,sizeof(CAPITAL),compare_dates);
}

void
Sort_by_Date(CAPITAL arr[], int nc)
{
    int i, j;
    CAPITAL temp;

    for (i = 0; i < nc - 1; i++) {
        for (j = i + 1; j < nc; j++) {
            int cmp = compare_dates(&arr[i],&arr[j]);
            if (cmp > 0) {
                temp = arr[i];
                arr[i] = arr[j];
                arr[j] = temp;
            }
        }
    }
}
Craig Estey
  • 30,627
  • 4
  • 24
  • 48
2

At least this problem:

Compare error

        if (month1_idx != month2_idx)
            return month1_idx < month2_idx;
        else if (day1 != day2)
            return day1 < day2;

        // This is never true as prior test is `month1_idx != month2_idx`
        if (month1_idx > month2_idx) {
            temp = arr[i];
            arr[i] = arr[j];
            arr[j] = temp;
        }
        ...
           if (day1 > day1) {  // This is never true.  Looks like a typo.

Instead:

        // If greater than ... 
        int gt = month1_idx > month2_idx || (month1_idx == month2_idx && day1 > day2);
        if (gt) {
          // swap
          ...
        }
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256