Filling vectors!

[ThePoloHobo]

So I'm trying to fill a vector with pointers to account objects.

std::vector<account*> fill_vector()
{
	std::vector<account*> temp;
	std::ifstream s;
	std::string str;
	s.open("H://account2.dat");
	std::string account_code, first_name, last_name;
	double balance;
	char type;
	
	for(int i = 0; i < 10; ++i) 
	{ 
		std::getline(s, str);
		std::istringstream(str) >> std::fixed >> std::setw(10)>> account_code >> first_name >> last_name >> type >> balance;
		account* A = factory(account_code, first_name, last_name, type, balance);
		if(A != nullptr){temp.push_back(A);}
	}
	return temp;
}	

The accounts have four different types and I am supposed to skip over ones that have an invalid account type and throw an exception:

account* factory(std::string account_code, std::string first_name, 
				 std::string last_name, char type, double balance)
{
try
{
	if(type == 'A')
	{
		simple_account *a = new simple_account(account_code, first_name, last_name, balance);
		return a;
	}
	else if(type == 'B')
	{
		bonus_account *a = new bonus_account(account_code, first_name, last_name, balance);
		return a;
	}
	else if(type == 'C')
	{
		service_account *a = new service_account(account_code, first_name, last_name, balance);
		return a;

	}
	else if(type == 'D')
	{
		service_account *a = new service_account(account_code, first_name, last_name, balance);
		return a;

	}
	else{
		throw myexception("Invalid account", 1);}
		account* a = nullptr;
		return a;
	}

	catch(myexception& e) {std::cerr<< e.get_description() << ": "<< e.get_code()<< std::endl;};

};

my problem is they program will not skip over my rejected accounts.. still adds them to the vector but I cant figure out why! Any help is greatly appreciated!!

[cire]

Lines 30 and 31 (of factory) cannot be executed because line 29 will always be executed. (I would be surprised if your compiler didn't warn you about unreachable code.)

Lose the exception.

account* factory(std::string account_code, std::string first_name,
	std::string last_name, char type, double balance)
{

	if (type == 'A')
		return new simple_account(account_code, first_name, last_name, balance);

	if (type == 'B')
		return new bonus_account(account_code, first_name, last_name, balance);

	if (type == 'C')
		return new service_account(account_code, first_name, last_name, balance);

	if (type == 'D')
		return new service_account(account_code, first_name, last_name, balance);

	return nullptr;
}

Do 'C' and 'D' really indicate the same type of account?

[ThePoloHobo]

No, that was some lazy copying and pasting lol..
Why was it only executing line 29?

I changed it to that but now it's saying my "vecotr subscript is out of range" at run time.

[ThePoloHobo]

I even changed my loop too:

for(std::vector<account*>::iterator i = temp.begin();i != temp.end(); ++i) 
	{ 
	        std::getline(s, str);
		std::istringstream(str) >> std::fixed >> std::setw(10)>>  account_code >> first_name >> last_name >> type >> balance;
		
                account* A = factory(account_code, first_name, last_name, type, balance);
		if(A != nullptr){temp.push_back(A);}
	}
	return temp;

still getting vector subscript out range.

In the debugger it breaks on the stdthrow.cpp in the VS library:

#ifdef _DEBUG
_CRTIMP2_PURE void __CLRCALL_PURE_OR_CDECL _Debug_message(const wchar_t *message, const wchar_t *file, unsigned int line)
	{	// report error and die
        if(::_CrtDbgReportW(_CRT_ASSERT, file, line, NULL, L"%s", message)==1)
        {
            ::_CrtDbgBreak(); //break here
        }
	}

[Disch]

When you push_back a vector... all existing iterators (in your case, 'i') potentially become invalid.

if(A != nullptr){temp.push_back(A);} // <- your 'i' iterator becomes invalid after this
 // push_back call.  So when you do ++i at the end of the loop, the program may explode. 

An alternative would be to loop by index, rather than by iterator.

[cire]

With for(std::vector<account*>::iterator i = temp.begin();i != temp.end(); ++i) since you start out with an empty vector, i == temp.begin() == temp.end() = No loop iteration. You'd be returning an empty vector from the function.

In the original loop there was also no indexing (therefore no possibility of an out-of-range subscript.) I'm reasonably sure that problem is in another chunk of code.

What do you do with the returned vector?

[ThePoloHobo]

ok, i switched it back but still getting the same error.
take a look at my entire "manager.h" manager class:

#include "factory.h"
#include <vector>
#include <iterator>

class manager
{
protected:
	std::vector<account*> a;

public:
	
	manager()
	{
		a = fill_vector();
	}

	~manager()
	{
		for(int i = 0; i < 10; ++i)
			delete a[i];
	}

	std::vector<account*> getVector() {return a;}

std::vector<account*> fill_vector()
{
	std::vector<account*> temp;
	std::ifstream s;
	std::string str;
	s.open("H://account2.dat");
	std::string account_code, first_name, last_name;
	double balance;
	char type;
	
	for(int i = 0; i < 10; ++i) 
	{ 
		std::getline(s, str);
		std::istringstream(str) >> std::fixed >> std::setw(10)>> account_code >> first_name >> last_name >> type >> balance;
		account* A = factory(account_code, first_name, last_name, type, balance);
		if(A != nullptr){temp.push_back(A);}
	}
	return temp;
}	

anything wrong here? like maybe calling the fill_vector function in the manager's constructor? This was the only way i could figure out how to get it constructed for use in main

as such:

#include "manager.h"

int main()
{
	manager manager1;
	std::vector<account*> a = manager1.getVector();
        manager1.update(a)
	return 0;
}

[cire]

~manager() is wrong. It should be:

    ~manager()
    {
        for ( unsigned i=0; i<a.size(); ++i )
            delete a[i] ;
    }

The constructor should probably be:

manager() : a(fill_vector()) {}

But, besides being slightly inefficient, there isn't anything wrong with the current constructor code.

[ThePoloHobo]

ahh it was the destructor messing me up! I thought initialization lists couldnt handle heavy processing because they are inlined?

Thanks!