Search code examples
c++arraysdynamicstructnew-operator

First attempt at using new to dynamically create struct array, program hangs without error


I'm trying to work out how to create a dynamic array for an assignment, but no matter what I seem to do my program continues to hang after I enter 3 or more entries.

I'm not getting any errors, and have to manually close the console window, but without any sort of direction I'm unsure of what exactly I've done wrong.

No matter what I change if I create 3+ structs in the console my program hangs after entering 'n' to end the creation of new Events.

int readEvents(Event* ev_ptr[], int size)
{
    char answer = 'y', slash;
    int i = 0;

    cout << "\nCreate an event [y/n]? ";
    cin >> answer;
    cin.ignore();
    while (answer == 'y' || answer == 'Y')
    {
        ev_ptr[i] = new Event;

        cout << "\nEnter description: ";
        cin.getline(ev_ptr[i]->desc, 80, '\n');

        cout << "\nEnter date: ";
        cin >> ev_ptr[i]->date.month >> slash >> ev_ptr[i]->date.day >> slash >> ev_ptr[i]->date.year;
        cin.ignore();

        cout << "\nEnter time: ";
        cin >> ev_ptr[i]->time.hour >> slash >> ev_ptr[i]->time.minute;
        cin.ignore();

        i++;

        cout << "\nCreate an event [y/n]? ";
        cin >> answer;
        cin.ignore();
    }

    return i;
}

Any help would be appreciated

Edit:

Here's my main function where the ev_ptr array size is declared:

int main()
{
    Event* event_pointers[100];
    int count = readEvents(event_pointers, 100), userMonth;
    char userString[80];

    cout << "\nEnter a search string: ";
    cin.getline(userString, 80, '\n');
    cin.ignore();

    linsearch(event_pointers, count, userString);

    cout << "\nEnter a month to list Events for: ";
    cin >> userMonth;
    cin.ignore();

    binsearch(event_pointers, count, userMonth);

    for (int j = 0; j < count; j++) //Cleanup loop
        delete event_pointers[j];

    cout << "\nPress any key to continue...";
    (void)_getch();
    return 0;
}

Solution

  • Using new and delete is somewhat the old way of doing things. Being the old way of doing things, there are decades worth of code making use of it. As summarized in my comments above, you currently declare 100 pointers-to-type Event, and then use new to allocate storage for each individual struct Event instance. By using a fixed array of pointers -- you are locked in to having no more events that you initially declare pointers for. That is a very inflexible way to go.

    Instead, simply declare a block of memory to hold some initial number of events, and keep a pair of counters for the number of elements allocated and number of elements filled. When filled == allocated, simply allocate another block of memory (say holding twice as many elements as the current), and copy the data from the original block to the new block, delete the original, and repeat as many times as required. (you are basically performing a manual realloc() using new/delete.

    A short example follows. In the example I use a simplified Event struct. (you can handle further splitting the dates and time into yyyy-mm-dd and H:M:S). The example starts by initially allocating 2 instances of Event and then continues reading as much input as the user has, updating the allocation size and copying original to new as required. The initial setup and reallocation code is as follows:

    #include <iostream>
    #include <cstring>
    
    #define MAXDESC  80    /* max description length */
    #define MAXDTTM  32    /* max date/time lengths */
    #define EVSIZE    2    /* initial number of Event elements allocated */
    
    struct Event {
        char desc[MAXDESC];
        char date[MAXDTTM];
        char time[MAXDTTM];
    };
    
    /** reallocate e to twice 'size' elements */
    Event *reallocEVx2 (Event *e, size_t *size)
    {
        Event *tmp = new Event[*size * 2];      /* allocate new block 2x in size */
    
        std::memcpy (tmp, e, *size * sizeof *e);    /* copy old to new */
        delete[] e;                                 /* delete old */
        *size *= 2;                                 /* update allocated size */
    
        return tmp;     /* return pointer to new block */
    }
    

    The readEvents() function takes the address-of your original pointer as a parameter, since the beginning address will change on reallocation. So instead of just passing a pointer, you pass the address-of the pointer from the caller so you can update that address to point to your newly reallocated block of memory in readEvents(). That change will be seen back in the caller because you are operating on the original pointer address. If you didn't pass the address-of the original pointer, you would have to return a pointer to the newly allocated block and assign that to your pointer back in the caller.

    The implementation for readEvents() does away with mixing getline and >> on std::cin. Instead simply use getline and create a stringstream from each line to further separate the data into individual values using >> on the stringstream. This eliminates the potential for an invalid input failing to be read and remaining unread in your input stream just waiting to bite you on your next input.

    A slight rewrite of your readEvents() simply loops taking input until the user types something other than 'y' when prompted:

    /** read events from user reallocating as required
     *  (note: must pass address-of-pointer, not just pointer)
     */
    size_t readEvents (Event **e, size_t *nelem, size_t *size)
    {
        char buf[MAXDESC];
    
        for (;;) {  /* loop continually until something other than 'y' entered */
            std::cout << "\nCreate event (y/n)? ";
            if (!std::cin.getline (buf, sizeof buf) || *buf != 'y')
                break;
    
            if (*nelem == *size)    /* check if realloc needed */
                *e = reallocEVx2 (*e, size);
    
            /* get input */
            std::cout << "  Enter description: ";
            if (!std::cin.getline ((*e)[*nelem].desc, MAXDESC))
                break;
    
            std::cout << "  Enter date: ";
            if (!std::cin.getline ((*e)[*nelem].date, MAXDTTM))
                break;
            /* create stringstream to parse date into yyyy-mm-dd here */
    
            std::cout << "  Enter time: ";
            if (!std::cin.getline ((*e)[*nelem].time, MAXDTTM))
                break;
            /* create stringstream to parse time into H:M:S here */
    
            (*nelem)++;     /* update no. of elements filled */
        }
        return *nelem;      /* return no. of elements filled */
    }
    

    In main(), you simply declare your counters to track the allocated number of elements (size below) and the number of elements used (nelem below) and allocate for your initial two instances of Event and starts reading data. The code finishes by outputting the memory statistics (allocated elements/used elements) and outputs each stored Event instance, e.g.

    int main (void) {
    
        size_t nelem = 0;
        size_t size = EVSIZE;
        Event *events = new Event[size];    /* allocate initial 'size' elements */
    
        if (!readEvents (&events, &nelem, &size)) {     /* read/fill events */
            std::cerr << "error: no events read.\n";
            return 1;
        }
    
        /* output memory useage (elements allocated/filled) */
        std::cout << "\nelements allocated: " << size <<
                     "\nelements filled   : " << nelem << "\n\n"; 
    
        for (size_t i = 0; i < nelem; i++)  /* output all events */
            std::cout << events[i].desc << "    " << events[i].date << "    " <<
                        events[i].time << '\n';
    
        delete[] events;    /* free memory  */
    }
    

    Putting it altogether, you could do:

    #include <iostream>
    #include <cstring>
    
    #define MAXDESC  80    /* max description length */
    #define MAXDTTM  32    /* max date/time lengths */
    #define EVSIZE    2    /* initial number of Event elements allocated */
    
    struct Event {
        char desc[MAXDESC];
        char date[MAXDTTM];
        char time[MAXDTTM];
    };
    
    /** reallocate e to twice 'size' elements */
    Event *reallocEVx2 (Event *e, size_t *size)
    {
        Event *tmp = new Event[*size * 2];      /* allocate new block 2x in size */
    
        std::memcpy (tmp, e, *size * sizeof *e);    /* copy old to new */
        delete[] e;                                 /* delete old */
        *size *= 2;                                 /* update allocated size */
    
        return tmp;     /* return pointer to new block */
    }
    
    /** read events from user reallocating as required
     *  (note: must pass address-of-pointer, not just pointer)
     */
    size_t readEvents (Event **e, size_t *nelem, size_t *size)
    {
        char buf[MAXDESC];
    
        for (;;) {  /* loop continually until something other than 'y' entered */
            std::cout << "\nCreate event (y/n)? ";
            if (!std::cin.getline (buf, sizeof buf) || *buf != 'y')
                break;
    
            if (*nelem == *size)    /* check if realloc needed */
                *e = reallocEVx2 (*e, size);
    
            /* get input */
            std::cout << "  Enter description: ";
            if (!std::cin.getline ((*e)[*nelem].desc, MAXDESC))
                break;
    
            std::cout << "  Enter date: ";
            if (!std::cin.getline ((*e)[*nelem].date, MAXDTTM))
                break;
            /* create stringstream to parse date into yyyy-mm-dd here */
    
            std::cout << "  Enter time: ";
            if (!std::cin.getline ((*e)[*nelem].time, MAXDTTM))
                break;
            /* create stringstream to parse time into H:M:S here */
    
            (*nelem)++;     /* update no. of elements filled */
        }
    
        return *nelem;      /* return no. of elements filled */
    }
    
    int main (void) {
    
        size_t nelem = 0;
        size_t size = EVSIZE;
        Event *events = new Event[size];    /* allocate initial 'size' elements */
    
        if (!readEvents (&events, &nelem, &size)) {     /* read/fill events */
            std::cerr << "error: no events read.\n";
            return 1;
        }
    
        /* output memory useage (elements allocated/filled) */
        std::cout << "\nelements allocated: " << size <<
                     "\nelements filled   : " << nelem << "\n\n"; 
    
        for (size_t i = 0; i < nelem; i++)  /* output all events */
            std::cout << events[i].desc << "    " << events[i].date << "    " <<
                        events[i].time << '\n';
    
        delete[] events;    /* free memory  */
    }
    

    Example Use/Output

    $ ./bin/events_new-del
    
    Create event (y/n)? y
      Enter description: Event 1
      Enter date: 11/29/19
      Enter time: 12:30:31
    
    Create event (y/n)? y
      Enter description: Event 2
      Enter date: 11/29/19
      Enter time: 12:30:41
    
    Create event (y/n)? y
      Enter description: Event 3
      Enter date: 11/29/19
      Enter time: 12:30:51
    
    Create event (y/n)? y
      Enter description: Event 4
      Enter date: 11/29/19
      Enter time: 12:31:01
    
    Create event (y/n)? y
      Enter description: Event 5
      Enter date: 11/29/19
      Enter time: 12:31:11
    
    Create event (y/n)? n
    
    elements allocated: 8
    elements filled   : 5
    
    Event 1    11/29/19    12:30:31
    Event 2    11/29/19    12:30:41
    Event 3    11/29/19    12:30:51
    Event 4    11/29/19    12:31:01
    Event 5    11/29/19    12:31:11
    

    Look things over and let me know if you have further questions about this approach.