Memory Exception

[Nw0DjE8b]

Hi, I'm back again with my mastermind program. It compiles, but when I attempt to run it, bad_alloc is thrown. I attempted to debug it with nothrow, but it's replaced by the backwards-incompatible __nothrow in Visual Studio. (Express 2012 on Windows 7) I know the problem occurs somewhere in the constructor of the menu class, but I can't determine the exact location. From what I've researched, bad_alloc is thrown when the program runs out of memory, which may somehow be happening in the loop. I don't delete the array here as I need it for another function. Though I do so in the destructor, I doubt it pertains to the issue as the object can't even get through it's own constructor, none the less go out of scope.

This is the menu class' constructor...

  menu::menu(int constructor_number_list_items, std::string title, std::string constructor_input_prompt, std::string constructor_bad_input, ...) {
    va_list prompt_data; /* in the form of:
                          *     - std::string : a handle string to the list item
                          *     - std::string : the list item
                          *     - int : the return value of "menu::return_input" if the input points to the respective list item
                          */
    number_list_items = constructor_number_list_items;

    prompt += title + "\n"; // The prompt will include the title...
    va_start(prompt_data, constructor_input_prompt);
    a_list_item_data_ptr = new list_item_data[number_list_items]; // initializing an array of list item data w/ an element for each list item

    for(int list_item = 0; list_item < number_list_items; list_item++) { // for each list item:
        std::string buffer; // the first argument of each list item

        buffer = va_arg(prompt_data, std::string);
        prompt += "    " + buffer + ".    "; // ...title, (looped) a 4-space indent, a handle string, a period, another 4-space indent...
        a_list_item_data_ptr[list_item].string_handle = buffer; /* initializing the string handle of the list item data of the respective list
                                                                 * item w/ the first argument
                                                                 */
        prompt += va_arg(prompt_data, std::string) + "\n"; // ...indent, and a list item.
        a_list_item_data_ptr[list_item].return_value = va_arg(prompt_data, int); // [...] the return value [...] the next argument
    }

    va_end(prompt_data);
    input_prompt = constructor_input_prompt;
    bad_input = constructor_bad_input;
}

...and this is how the first object is initialized:

  menu main_menu(3, "Main Menu:", "input: ", "    error : invalid input\n\n", 1, "New Game", 1, 2, "Exit", 2, 3, "About", 3); // an object of "menu" for the main menu 

Part of the header file:

  struct list_item_data {
        std::string string_handle;
        int return_value;
    };

    list_item_data * a_list_item_data_ptr;

// ...

menu(int, std::string, std::string, std::string, ...);

I also tried using smart pointers, but with little success, as I was unable to find the header file.

Also, about the destructor, is this the correct way to free the memory?

  menu::~menu() {
    for(int element = 0; element < number_list_items; element++)
        delete &(a_list_item_data_ptr[element]);

    delete[] a_list_item_data_ptr;
}

Thanks.

[Lowest0ne]

Also, about the destructor, is this the correct way to free the memory?

No, all that is needed is line 5 from that code.

I think that would solve your problem, not sure. Nothing looks like you are allocating a huge amount of memory.

You can change the way you handle this program to make it much simpler. The first would be to get rid of the variable length args ( something you should not be using in the first place ). You can replace it with a vector:

#include <vector> // for the vector, an array with changeable size
#include <string>
#include <iostream>

int main(void)
{
    // if you have c++11
    // std::vector < std::string> choices = { "First", "Second", "Third" };
    // else
    std::vector< std::string >  choices;
    choices.push_back(  "First" );
    choices.push_back( "Second");
    choices.push_back( "Third" );

    std::cout << "Please make a choice: ";

    // vectors know their own size
    // they also have a "size_type" which indicates the indexing type 
    for ( std::vector::size_type i = 0 ; i < choices.size() ; ++i)
    {
        std::cout << i + 1 << ") " << choices[i] << std::endl;
    }

    int choice;
    std::cin >> choice;  std::cin.ignore(80, \n ); //needs single quotes
    choice--;  // to align it with vector indicies
   
    // TODO: make sure choice is valid
    
   std::cout << "You selected: " << choices[choice] << std::endl;

  return 0;
}

Have your menu store a vector of choices (strings) instead of the pointer. You dont need memory allocation at all that way.

[Nw0DjE8b]

Thanks, especially for your insight on using vectors, but editing the destructor didn't work. However, I suspect the error has to do with the variable argument list, so including vectors might fix it. It may take me a little while, so I'll get back to you when I've made some progress.

[Nw0DjE8b]

Thanks, using vectors fixed my problem! I still have a few bugs to iron out, but nothing pertaining to memory.

One thing I'm wondering - can I declare structs from within a function call? I've set up a vector of structs, (containing the data I would've entered into a va_list) but initializing it right now requires I define temporary struct variables, which seems very inefficient.

[Lowest0ne]

The way you have it, you don't need variable argument lists at all:

#include <iostream>
#include <vector>
#include <string>

class Vector_Store
{
	private:
	std::vector< std::string > m_data;
	std::string m_title;
	
	public:
	
	Vector_Store( const std::vector< std::string > & data,
			      const std::string& title = "default title"
	)
		: m_data(data), m_title(title)  // initializer lists are your friend
	{
		// don't need anything
	}
	
	int get_choice(void) const  // const says we won't modify any members here
	{
		std::cout << m_title << std::endl;
		
		for (std::vector<std::string>::size_type i = 0; i < m_data.size(); i++)
		{
			std::cout << '\t' << i + 1 << ": " << m_data.at(i) << std::endl;
		}
		
		// refer to earlier post for data input
		
		return 0; // return choice in real program
	}
};

int main(void)
{
	std::vector<std::string> list;
	list.push_back("first");
	list.push_back("second");
	
	Vector_Store store_1(list);  // with default title
	Vector_Store store_2(list, "custom title");
	
	store_1.get_choice();
	store_2.get_choice();
	
	return 0;
}