CODEWITHSUNDEEP
c++arraysclassstruct
JeffR
JeffR

Reputation: 805

How to convert struct with array to class?

Here's what I have:

void function() {
struct APROCS
{
DWORD PID;
TCHAR processname[256];
};  
static struct APROCS *aProcesses;
aProcesses = (APROCS *) calloc(8192, sizeof(APROCS));    

//do work...
[get PIDs and process names]
if ((aProcesses[i].PID > 4) && (_tcslen(aProcesses[i].processname) > 0
//do more work...    

free(aProcesses);aProcesses=NULL;
}

I'm looking to eliminate all bad code, and while this code works, it's probably not the correct way to do it. I basically setup an array of aProcesses[8192]. I want to use a class:

// CProcesses.h 
class CProcesses
{
public:
DWORD PID;
TCHAR processname[256];

CProcesses(void);
~CProcesses(void);
};

and

// CProcesses.cpp   
#include <Windows.h>
#include "CProcesses.h"
CProcesses::CProcesses(void)
{
}
CProcesses::~CProcesses(void)
{
}

Do I do this to allocate an array from the class?

void function() {
CProcesses *aProcesses[8192];

//do work...
[get PIDs and process names]
if ((aProcesses[i].PID > 4) && (_tcslen(aProcesses[i].processname) > 0
//do more work...  
}

Upvotes: 0

Views: 217

Answers (2)

xen-0
xen-0

Reputation: 709

I'm looking to eliminate all bad code, and while this code works, it's probably not the correct way to do it.

So what's wrong with it? What needs to be fixed. It's not "bad" just because it uses the keyword struct instead of class. You can't fix broken code without knowing why it's broken.

A struct in C++ is simple a class where members are public by default, rather than private. So as is, your alternative implementation isn't any different.

You might regard the code as "bad" because it has a manual memory allocation with calloc that has to be explicitly freed. If an exception is thrown between your calls to calloc and free, you'll leak memory.

Since this is C++, why not keep the struct for the process (it's typical in C++ to use a struct for a pure lump of data with no methods) and use a class to manage the collection? You can either use a std::vector instead of a primitive array, or write your own class to manage the memory, allocating it in the constructor and freeing it in the destructor.

Using a vector is simplest and your less likely to make mistakes.

On the subject of vectors, it's safest to use STL containers as much as you can. e.g. use std::strings instead of char or TCHAR arrays/pointers for as long as possible, and only use the underlying char array when you need to pass it to the Win32 API.

Upvotes: 2

utnapistim
utnapistim

Reputation: 27365

You should probably replace APROCS::processname with a std::string.

If your application is to be compiled only with TCHAR=char, you can use std::string. If it is to be compiled with TCHAR=wchar_t, consider using std::wstring.

You should also replace aProcesses with a std::vector.

This is not mandatory, but probably should make your data private in CProcesses and use accessors for it (or leave it all as a struct with public data.

Upvotes: 0

Related Questions