Class Vectorization requirements

[Drowzee]

My project continues... But at least I'm getting better at using C/C++ and MFC.

Now, I'm at a critical point: dynamic memory allocation fundamentals.

I have a data class with a number of unsigned char arrays and longs that are addressed in structs (encapsulated by the class).

I'd like to make a vector of this data class. I have no use for the '==' and '<' operators, but I'll overload them anyway, but my problem is this:
I've got long arrays of unsigned chars (1000+spaces), and I intend to have about 250 different instances of this class in memory during regular operation.

How do I make a constructor for those unsigned char arrays? Should I make a loop, or can I just initialize the first element of an array and call it good?

Additionally, the file where this is declared is #included by two files that I'm intentionally keeping apart for ease of coding.

I'm using the

#ifndef
#def


#endif

directives to isolate the class to one defintion, but for some reason...
If I put this class in there, only the class itself is protected by the preprocessor. If I put the class constructor outside the terminating bracket for the class, I get "already defined in suchandsuch.obj" errors.

And it's the same with the overloaded operator functions.

How can I fix these problems?

Placing the overloads inside the class seems to fix the redefinition problem, but they now claim that the binary operator < has too many parameters. Doesn't 'bi' translate to two? As in, the two parameters that I'm trying to explicitly overload?

[Ancient Dragon]

you are using c++ -- why are you mixing c-style char arrays when you could be using std::string class that will do all the allocation for you. Post your class and we'll see how to get rid of those char arrays!

[Drowzee]

Ahh, yes. I knew that question was on the way.

Problem is: I'm an intern. Working with what should be considered legacy code, because it's from a tool that's several years old. The work is with ethernet packets, stripping headers and using sizeof() in a fascinating way to collect the data in c-style file operations. It works, I've gotten it working for a single packet, and I want to get this GUI working without gutting the entire code by switching all the C over to C++.

That's mainly because the original was written in C that was way over my head, and deals with the company's proprietary technology. I feel like I should leave things more or less as I found them, so that when I'm gone, this tool will still be used and maintainable by people who know (drumroll, please!) C much more strongly than C++.

If it were my project entirely, and not something I was doing for a resume, sure, I'd post the code and switch everything to C++. But as it stands, I am rather leery of making such a fundamental change in the short amount of (paid) time remaining.

[Ancient Dragon]

How do I make a constructor for those unsigned char arrays? Should I make a loop, or can I just initialize the first element of an array and call it good?

c-style char arrays do not have constructors. you must allocate the memory in the container class. Are the arrays pointers what need to be allocated? or arrays of hard-coded size?

class myclass
{
...
   char* array;
   // or
   char array[1000];
}

directives to isolate the class to one defintion, but for some reason...If I put this class in there, only the class itself is protected by the preprocessor. If I put the class constructor outside the terminating bracket for the class, I get "already defined in suchandsuch.obj" errors.

And it's the same with the overloaded operator functions.

you don't put executable code (except inline code) in header files for the very reason that you state -- duplicate functions. Instead, put the class declaration inside the defines and put the implementation code in a *.cpp file. Same for overloaded operators.

You need to post some code before anyone can give you exact answers to your questions. Here is an example:

// myclass.h
#ifndef _MYCLASS_H
#define _MYCLASS_H
class myclass
{
public:
   myclass();
   virtual ~myclass();
  // overload operator
   bool operator<(const char* str);
   bool operator==(const char* str);
   // global overloaded operator that is not part of the class
   friend bool operator<(myclass& mc,const char*s);

protected:
  char array[1024];
};
#endif // _MYCLASS_H

// myclass.cpp
myclass::myclass()
{
   // initialize array
   memset(array,0,sizeof(array));
}


bool myclass::operator<(const char*s)
{
  return (strcmp(array,s) < 0) ? true : false;
}

bool operator<(myclass& mc,const char* s)
{
   return strcmp(mc.array,s) == 0 ? true : false;
}

[Drowzee]

After an intial success, the vector is giving me problems. So, since I can show this public-domain info...

#ifndef CPackethdr
#define CPackethdr
class CPacket //Encapsulates data structs and associated information for easier instantiation.
{
public:
	CPacket();
	virtual ~CPacket();

	bool operator<(const char* str);   //Included for the purpose of vectorization, but not used.
	bool operator==(const char* str);

	struct PeekPacket7
	{
		unsigned short	fProtoSpec;
		unsigned short	fPacketLength;
		unsigned short	fSliceLength;
		unsigned char	fFlags;
		unsigned char	fStatus;
		unsigned long	fTimeStampHi;
		unsigned long	fTimeStampLo;
	}	ppacket;
	
	struct PacketHeader
	{	
		unsigned char	fDestAddr[6];
		unsigned char	fSrcAddr[6];
		unsigned short	fProtocol;
		unsigned char	fPacketData[1500];  /* max packet size */
	}   enetpacket;
	
	int						length;
	unsigned long			packetCount;
	char					timeString[16];
		
};
#endif

With the following in the .cpp...

CPacket::CPacket()
{
	ppacket.fProtoSpec = 0;
	ppacket.fPacketLength = 0;
	ppacket.fSliceLength=0;
	ppacket.fFlags = 0;
	ppacket.fStatus = 0;
	ppacket.fTimeStampHi= 0L;
	ppacket.fTimeStampLo= 0L;
	
	memset(enetpacket.fDestAddr, 0, sizeof(enetpacket.fDestAddr));
	memset(enetpacket.fSrcAddr,0,sizeof(enetpacket.fSrcAddr));
	enetpacket.fProtocol= 0;
	memset(enetpacket.fPacketData,0,sizeof(enetpacket.fPacketData));
	
	length = 0;
	packetCount = 0;
	memset(timeString,0,sizeof(timeString));
}

CPacket::~CPacket()
{}


bool CPacket::operator<(const char* str)
{
	return TRUE;
}

bool CPacket::operator==(const char* str)
{
	return TRUE;
}

And I'd like to declare the vector in another class in the same header file. Why? Because this is an MFC application, and doesn't have a 'main' function to make things simple to place.
When I call

vector<CPacket> packlist(40);

In the second class of the header file, the compiler lists the following errors:

error C2143: syntax error : missing ';' before '<'
error C2501: 'vector' : missing storage-class or type specifiers
error C2059: syntax error : '<'
error C2238: unexpected token(s) preceding ';'

Is this because of my friendless operator overloading?

[Dave Sinkula]

Did you #include <vector>?

[Ancient Dragon]

here's an easier way to initialize the structures. It's not necessary to initialize each field individually, just flood the entire structure with 0s.

memset(&ppacket,0,sizeof(ppacket));
    memset(&enetpacket,0,sizeof(enetpacket));

In MFC application you can put that vectory array in any of the MFC classes -- most probably either CWin-derived or CDocument-derived class, depending what all needs access to the vector. you may not be able to initialize the vector's size like you posted, but do it in two stages:

class CMyDocument : public CDocument
{
  ...
private:
   vector<CPacket> packlist;
};

// class constructor
CMyDocument::CMyDocument()
{
   packlist.resize(40);


};

[Drowzee]

Thanks for the advice regarding the initalizers. That certainly cleans things up nicely

Did you #include <vector>?

Yes.

And the vector declaration is in the CDocument derived class, which is why I'm confused.

I have tried initalizing the vector in the class then resizing, and it doesn't work.
When I try to put the declaration in the CDoc derived class constructor, it gives the errors:

error C2065: 'vector' : undeclared identifier
error C2275: 'CPacket' : illegal use of this type as an expression
see declaration of 'CPacket'
error C2065: 'packlist' : undeclared identifier

However: when I wrote the vector call, the microsoft tooltip/little helperbox came up with:

<typename _TY, typename _A=allocator<_TY>>

and could respond to my request to go to the defintion of vector (though it brought up a dialog box to resolve ambiguity).
I think that means it knows dang well what I want.

[Ancient Dragon]

works for me. Maybe your problem is that you did not include CPacket.h in the document.h. I just put them in the same file, but you can also have them in two different files and include cpacket.h in the other .h.

#if !defined(AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_)
#define AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_

#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000

#include <vector>

class CPacket //Encapsulates data structs and associated information for easier instantiation.
{
public:
	CPacket();
	virtual ~CPacket();

	bool operator<(const char* str);   //Included for the purpose of vectorization, but not used.
	bool operator==(const char* str);

	struct PeekPacket7
	{
		unsigned short	fProtoSpec;
		unsigned short	fPacketLength;
		unsigned short	fSliceLength;
		unsigned char	fFlags;
		unsigned char	fStatus;
		unsigned long	fTimeStampHi;
		unsigned long	fTimeStampLo;
	}	ppacket;
	
	struct PacketHeader
	{	
		unsigned char	fDestAddr[6];
		unsigned char	fSrcAddr[6];
		unsigned short	fProtocol;
		unsigned char	fPacketData[1500];  /* max packet size */
	}   enetpacket;
	
	int						length;
	unsigned long			packetCount;
	char					timeString[16];
		
};

class CTry5Doc : public CDocument
{
protected: // create from serialization only
	CTry5Doc();
	DECLARE_DYNCREATE(CTry5Doc)

// Attributes
public:
	std::vector<CPacket> theList;

// Operations
public:

// Overrides
	// ClassWizard generated virtual function overrides
	//{{AFX_VIRTUAL(CTry5Doc)
	public:
	virtual BOOL OnNewDocument();
	virtual void Serialize(CArchive& ar);
	//}}AFX_VIRTUAL

// Implementation
public:
	virtual ~CTry5Doc();
#ifdef _DEBUG
	virtual void AssertValid() const;
	virtual void Dump(CDumpContext& dc) const;
#endif

protected:

// Generated message map functions
protected:
	//{{AFX_MSG(CTry5Doc)
		// NOTE - the ClassWizard will add and remove member functions here.
		//    DO NOT EDIT what you see in these blocks of generated code !
	//}}AFX_MSG
	DECLARE_MESSAGE_MAP()
};

/////////////////////////////////////////////////////////////////////////////

//{{AFX_INSERT_LOCATION}}
// Microsoft Visual C++ will insert additional declarations immediately before the previous line.

#endif // !defined(AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_)

BOOL CTry5Doc::OnNewDocument()
{
	if (!CDocument::OnNewDocument())
		return FALSE;
	theList.resize(40);
	// TODO: add reinitialization code here
	// (SDI documents will reuse this document)

	return TRUE;
}

[Drowzee]

Hm. Looks like the addition of std:: was what it wanted.
Sorry about being such a stubborn sumbit with giving out the information...

Frikkin' Sweet. It's working.

[Drowzee]

One last question:

One of the employees informed me that nesting structs inside the class was not good coding practice. How should I clean it up?
Should I:
Make new classes from the structs
Typedef the structs
or do something else?

I prefer the way it is right now, because it makes sense for a class called 'Packet' to have the full packet representation and all associated information...

[Dave Sinkula]

One of the employees informed me that nesting structs inside the class was not good coding practice. How should I clean it up?

Here is my first reaction to the suggestion.

Instead of this:

#if !defined(AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_)
#define AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_

#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000

#include <vector>

class CPacket //Encapsulates data structs and associated information for easier instantiation.
{
public:
	CPacket();
	virtual ~CPacket();

	bool operator<(const char* str);   //Included for the purpose of vectorization, but not used.
	bool operator==(const char* str);

	struct PeekPacket7
	{
		unsigned short	fProtoSpec;
		unsigned short	fPacketLength;
		unsigned short	fSliceLength;
		unsigned char	fFlags;
		unsigned char	fStatus;
		unsigned long	fTimeStampHi;
		unsigned long	fTimeStampLo;
	}	ppacket;
	
	struct PacketHeader
	{	
		unsigned char	fDestAddr[6];
		unsigned char	fSrcAddr[6];
		unsigned short	fProtocol;
		unsigned char	fPacketData[1500];  /* max packet size */
	}   enetpacket;
	
	int						length;
	unsigned long			packetCount;
	char					timeString[16];
		
};

Do this:

#if !defined(AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_)
#define AFX_TRY5DOC_H__BA63042D_EFBE_45CF_A864_A01C9FB8E010__INCLUDED_

#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000

#include <vector>

struct PeekPacket7
{
   unsigned short	fProtoSpec;
   unsigned short	fPacketLength;
   unsigned short	fSliceLength;
   unsigned char	fFlags;
   unsigned char	fStatus;
   unsigned long	fTimeStampHi;
   unsigned long	fTimeStampLo;
};

struct PacketHeader
{	
   unsigned char	fDestAddr[6];
   unsigned char	fSrcAddr[6];
   unsigned short	fProtocol;
   unsigned char	fPacketData[1500];  /* max packet size */
};

class CPacket //Encapsulates data structs and associated information for easier instantiation.
{
public:
	CPacket();
	virtual ~CPacket();

	bool operator<(const char* str);   //Included for the purpose of vectorization, but not used.
	bool operator==(const char* str);

	struct PeekPacket7  ppacket;
	struct PacketHeader enetpacket;
	
	int						length;
	unsigned long			packetCount;
	char					timeString[16];
		
};

Or further move things by defining the structs in their own headers and #include them.

[Drowzee]

Would it be worthwhile to typedef them as well?
I use sizeof later on in grabbing information, so... might as well simplify the call, correct?

[Dave Sinkula]

Would it be worthwhile to typedef them as well?

In C++ that's already done automatically. And in C I prefer not to.

I use sizeof later on in grabbing information, so... might as well simplify the call, correct?

Why? Using sizeof on the object rather than it's presumed type is safer anyway.

[Drowzee]

Okay. It didn't require much of a change from the original implementation that way...
Thanks, Dave.

[Ancient Dragon]

One last question:
One of the employees informed me that nesting structs inside the class was not good coding practice. How should I clean it up?

that's a matter of opinion, program design, and corporate coding standards. coding a structure inside a class limits the structure's scope to that class. I, for one, see nothing wrong with that and have used it myself.