Issues with Vector iterator pointing to a parent class pointer from an inherited one

[BruceJohnJennerLawso]

Okay, so this is a bit of a tricky issue.

I'm trying to write a sort of resource that will make creating command line programs a bit easier. To start off, I created a base class for generic menu types

#pragma once
#include <iostream>
#include <string>
#include <vector>
#include <string>
#include <vector>
#include "FRepo.h"
#define NEWLINE '\n'
#define TAB '\t'
#define VTAB '\v'
#define BEEP '\a'
#define BACK '\b'
using namespace std;

class CGenericMenu
{	public:
	enum rel {up, down, sideways, root};
	CGenericMenu(void);
	CGenericMenu(CGenericMenu * MAddressup, CGenericMenu * MAddressdown, string mid, string mrf);
	virtual void menu();
	virtual void menuhelp();
	~CGenericMenu(void);	
	string menuid;
	string menuref;
	vector<CGenericMenu*> Linkslist;
										};

	vector<CGenericMenu*>::iterator it;

#include "GenericMenu.h"
#include <iostream>
#include <string>
#include <vector>
#include "FRepo.h"
#define NEWLINE '\n'
#define TAB '\t'
#define VTAB '\v'
#define BEEP '\a'
#define BACK '\b'
using namespace std;

CGenericMenu::CGenericMenu(void)
{					}

CGenericMenu::CGenericMenu(CGenericMenu * MAddressup, CGenericMenu * MAddressdown, string mid, string mrf)
{	menuid = mid;
	menuref = mrf;
	Linkslist.emplace(Linkslist.begin(), (MAddressup));
	Linkslist.emplace(Linkslist.begin(), (MAddressdown));

}

CGenericMenu::~CGenericMenu(void)
{					}

But I figured that it would make more sense to use that as a base class, then derive a command line-specific one from it (all of my work so far is Command line). So I wrote a class for that...

#pragma once
#include "genericmenu.h"
#include <iostream>
#include <string>
#include <vector>
#include "FRepo.h"
#define NEWLINE '\n'
#define TAB '\t'
#define VTAB '\v'
#define BEEP '\a'
#define BACK '\b'
using namespace std;

class TCmdMenu :public CGenericMenu
{	public:
	TCmdMenu(void);
	void menu();
	~TCmdMenu(void);
				

						};

#include "TCmdMenu.h"
#include <iostream>
#include <string>
#include <vector>
#include "FRepo.h"
#define NEWLINE '\n'
#define TAB '\t'
#define VTAB '\v'
#define BEEP '\a'
#define BACK '\b'
using namespace std;

TCmdMenu::TCmdMenu(void)
{				}

void TCmdMenu::menu()
{	cout << "--" << menuid << "--" << NEWLINE << NEWLINE;
	string i;
	i = Input_string("Please input menu id: ");	
	for (it = (Linkslist.end()-1); it != (Linkslist.begin()-1); --it)
	{	string cmp;
		it->
		Compare_string(i, cmp);
													
											}			}

void CGenericMenu::menuhelp()
{				}


TCmdMenu::~TCmdMenu(void)
{				}

Obviously I want to use the iterator to work through each member in linkslist, a member vector of the base class that holds pointers to the menus around each menu. The problem appears to be when I try to use the iterator it to access the elements of the member it is accessing. The compiler keeps telling me that the expression must have pointer to class type.

Can anyone shed a bit of light on the situation? My brain is thoroughly knotted from wrestling with this.

Thanks in advance.

[ne555]

First, ¿why do you have an iterator as a member of your class? Try to limit the scope of your variables.

vector<CGenericMenu*>::iterator it; The container holds pointers, if you dereference the iterator you'll obtain a pointer
(*it)->menu();

By the way, if you've got at least one virtual member function, then you should have a virtual destructor.


Also, ¿what's with the weird indentation?

[BruceJohnJennerLawso]

Bingo, that did it, thank you :)

I had tried using the *it, but I never enclosed it in brackets.

Virtual Destructors... Gotta check my notes again, oh well...

Oh, and the indentation is just my thing. Allows for the same amount of code onscreen at any time, but much easier to read (at least IMHO). I always hated the standard

function {
statements
}

layout because its disjointed visually

function
{ statement1
statement2 }

allows me to draw a little box around it, using the curly brackets as the points of the rectangle. But I digress...

Thanks again :)

Edit: One last thing. The reason that the iterator was a member was because each menu needed to have an individual list of menus that it could link to. Having the vector externally wouldn't make sense from this perspective.

[ne555]

> Having the vector externally wouldn't make sense from this perspective.
I am not complaining about the vector being a member, I complain about the iterator.

Wait... the iterator is not a member, but a global variable. That's even worst.

[BruceJohnJennerLawso]

Yes, I had to change the structure later on, now the vector and the iterator are external. (I have no idea why I thought the vector had to be menu specific when I was already passing strings to each menu to be compared). :facepalm:

Why exactly is a global iterator a problem?

Just for the record, the code is working beautifully now. I used it to create a wav player in ~3-4 lines yesterday. I can post it if you want to see it.

[ne555]

http://c2.com/cgi/wiki?GlobalVariablesAreBad (I've just read the first section)
Now consider your code for (it = (Linkslist.end()-1); it != (Linkslist.begin()-1); --it)
¿what advantage do you have by making `it' a global?
Please note that you don't care about its previous value, and you left it in an invalid state


By the way, if you want to traverse on the other direction, consider using a reverse iterator
for(auto it = Linkslist.rbegin(); it != Linkslist.rend(); ++it)

PS: `Linkslist' for a vector is confusing

[BruceJohnJennerLawso]

Okaaay, fair enough, the article does make some good points, but I still dont see any advantage to making the iterator local to each menu object. All that would do would add another minor amount of memory allocation to each object for each menu's interator, when the tasjk at hand only requires one allocator to function anyways?

Ie, why grow memory usage overhead linearly with the number of menu objects, when it could be constant instead? (at least wrt Iterators)

[cire]

Okaaay, fair enough, the article does make some good points, but I still dont see any advantage to making the iterator local to each menu object.

You're missing the point. The iterator should not be local to each menu object. It should only be present in the function that uses it. There is no global object. There is no extra memory per object.

[BruceJohnJennerLawso]

Ah good point. I'll make the necessary changes then.

Thanks for all of the help on this. :)