A Better Class Design

[Bryan Parkoff]

I have created three classes according to my own design. First class
is called CMain. It is the Top Class. Second class and third class are
called CMemory and CMPU. They are the sub-classes.
Two sub-classes have the relationship to communicate back and forth
through this pointer. The pointer is responsible inside Top class for
allocating and deallocating two sub-classes.
CMemory class is responsible to allocate and deallocate memory while
CMPU class does not. I tell the pointer to copy m_pMemoryRegister's
memory address from CMemory class to CMPU class. It is done once during
the initialization.
After the initialization is done, the functionality can perform all
the tasks inside CMPU class rather than depending on CMemory class so it
can gain a big impact of this performance.
I understand that some C programmers prefer to leave all variables
and functions in the global. In fact, it won't be efficiency because all
variables and functions have fixed memory that they are stored in data
segment. They can't perform the tasks into 80x86's L1 Cache.
It is best to store both variables and functions inside class because
they can be very efficiency. They are stored in stack segment. They can
be able to perform the tasks into 80x86's L1 Cache.
Please read my simple code below. Please state your opinion to see
if my programming code for class is a good design. I appreciate that you
can provide what you think is best.


class CMain
{
public:
CMain();
virtual ~CMain();

CMemory* Get_Memory();
CMPU* Get_MPU();

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMemory* m_pMemory;
CMPU6502* m_pMPU;
};

CMain::CMain()
{
m_pMemory = new CMemory(this);
m_pMPU = new CMPU(this);
}

CMain::~CMain()
{
delete m_pMemory;
delete m_pMPU;
}

CMemory* CMain::Get_Memory()
{
return m_pMemory;
}

CMPU* CMain::Get_MPU()
{
return m_pMPU;
}

void CMain::Initialize(void)
{

}

void CMain::Terminate(void)
{

}

void CMain::Run(void)
{
m_pMPU->Run();
}

class CMemory
{
public:
CMemory();
virtual ~CMemory();

CMemory(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

unsigned char Get_MemoryRegister(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

CMemory::CMemory()
{
Initialize();
}

CMemory::~CMemory()
{
Terminate();
}

CMemory::CMemory(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMemory::Initialize(void)
{
m_pMemoryRegister = new unsigned char [0x10000];
memset(&m_pMemoryRegister[0], 0x00, 0x10000);
}

void CMemory::Terminate(void)
{
delete [] m_pMemoryRegister;
}

void CMemory::Run(void)
{

}

PU_BYTE CMemory::Get_MemoryRegister(void)
{
return m_pMemoryRegister;
}


class CMPU
{
public:
CMPU();
virtual ~CMPU();

CMPU(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

CMPU::CMPU()
{
Initialize();
}

CMPU::~CMPU()
{
Terminate();
}

CMPU::CMPU(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMPU::Initialize(void)
{
m_pMemoryRegister = m_pMain->Get_Memory()->Get_MemoryRegister();
}

void CMPU6502::Terminate(void)
{

}

void CMPU6502::Run(void)
{

}

int main(void)
{
CMain Main;
Main.Run();

return 0;
}

 

[Steven T. Hatton]

I have created three classes according to my own design. First class
is called CMain. It is the Top Class. Second class and third class are
called CMemory and CMPU. They are the sub-classes.
Two sub-classes have the relationship to communicate back and forth
through this pointer. The pointer is responsible inside Top class for
allocating and deallocating two sub-classes.
CMemory class is responsible to allocate and deallocate memory while
CMPU class does not. I tell the pointer to copy m_pMemoryRegister's
memory address from CMemory class to CMPU class. It is done once during
the initialization.
After the initialization is done, the functionality can perform all
the tasks inside CMPU class rather than depending on CMemory class so it
can gain a big impact of this performance.
I understand that some C programmers prefer to leave all variables
and functions in the global. In fact, it won't be efficiency because all
variables and functions have fixed memory that they are stored in data
segment. They can't perform the tasks into 80x86's L1 Cache.
It is best to store both variables and functions inside class because
they can be very efficiency. They are stored in stack segment. They can
be able to perform the tasks into 80x86's L1 Cache.
Please read my simple code below. Please state your opinion to see
if my programming code for class is a good design. I appreciate that you
can provide what you think is best.

I find the code interesting, but I'm not an expert in C++. I don't fully
understand what you are suggesting. Are you trying some kind of ipc, or
running your own memory manager?

 

[Pete Vidler]

Bryan Parkoff wrote:
[snip confusing text]

Please read my simple code below. Please state your opinion to see
if my programming code for class is a good design. I appreciate that you
can provide what you think is best.

I've listed a few problems with the code inline below. Note that I
couldn't make much sense of your main text, so I might be missing the point.

class CMain
{
public:
CMain();
virtual ~CMain();

A virtual destructor usually indicates that this is meant to be a base
class. If you follow the advice of Scott Meyers ("More Effective C++"
item 33) then base classes (and all non-leaf classes) should be abstract.

Even if you don't follow that advice, I see no reason for a virtual
destructor unless you have specific extension scenarios in mind (which
you might, but I didn't see them).

CMemory* Get_Memory();
CMPU* Get_MPU();

If you /must/ return raw pointers (and I don't recommend it), then at
least provide const versions of your get methods (with const return
values, obviously). Then your code will still be usable within other
const methods.

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMemory* m_pMemory;
CMPU6502* m_pMPU;

There are many problems with raw pointers. You might consider smart
pointers, such as from the boost or loki libraries.

};

CMain::CMain()
{
m_pMemory = new CMemory(this);
m_pMPU = new CMPU(this);
}

What happens if one of those throws an exception? In general, you should
keep one resource to a class to avoid these issues (or use smart pointers).

CMain::~CMain()
{
delete m_pMemory;
delete m_pMPU;
}

Where is the copy constructor and copy assignment operator? If you don't
have them the compiler will generate them for you. The compiler
generated versions will not work properly for this class... they will
simply copy the pointer and not the data it points to. Then you call
'delete' on those pointers twice (from each destructor)...

Even if CMain is not meant to be copied, you should make those two
methods private (or declare them, but don't implement them). This stops
people from making mistakes with your code.

[snip rest of code]

Maybe if you include a description of exactly what you were trying to
do, rather than a description of the classes you have?

-- Pete

 

[Siemel Naran]

I have created three classes according to my own design. First class
is called CMain. It is the Top Class. Second class and third class are
called CMemory and CMPU. They are the sub-classes.
Two sub-classes have the relationship to communicate back and forth
through this pointer. The pointer is responsible inside Top class for
allocating and deallocating two sub-classes.

Then the copy constructor and operator= of the sub-classes should probably
be private and not implemented to prevent users from copying one CMemory or
CMPU object to another. Alternatively choose the design in the next
paragraph.

On the other hand, if you clone one of these objects, which CMain does the
cloned object point to? This is an important question, because when the
CMain goes out of scope it destroys the CMemory and CMPU objects it owns, so
if two CMemory objects can point to the same CMain, the CMain has to have an
array of pointers to CMemory objects (not just one pointer as in your class
below).

CMemory class is responsible to allocate and deallocate memory while
CMPU class does not. I tell the pointer to copy m_pMemoryRegister's
memory address from CMemory class to CMPU class. It is done once during
the initialization.

Why does CMPU not own the CMemory class? I think I see the reason: 2 CMPU
objects may share the same memory, right?

After the initialization is done, the functionality can perform all
the tasks inside CMPU class rather than depending on CMemory class so it
can gain a big impact of this performance.
I understand that some C programmers prefer to leave all variables
and functions in the global. In fact, it won't be efficiency because all
variables and functions have fixed memory that they are stored in data
segment. They can't perform the tasks into 80x86's L1 Cache.
It is best to store both variables and functions inside class because
they can be very efficiency. They are stored in stack segment. They can
be able to perform the tasks into 80x86's L1 Cache.
Please read my simple code below. Please state your opinion to see
if my programming code for class is a good design. I appreciate that you
can provide what you think is best.


class CMain
{
public:
CMain();
virtual ~CMain();

CMemory* Get_Memory();
CMPU* Get_MPU();

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMemory* m_pMemory;
CMPU6502* m_pMPU;
};

Since CMain owns the CMemory and CMPU objects, consider using a smart
pointer like std::auto_ptr<CMemory>. You get exception safety in the
constructor, as noted below. Note that if your CMain.h header file does not
include CMemory.h and CMPU.h, you must declare the destructor in the header
file as above, and define it in the cpp file with an empty function body.
You should consider declaring the copy constructor and operator= of CMain
private and not implemented.

In all your classes, Initialize and Terminate are helper functions called
from the constructor and destructor, so make them private. Alternatively,
maybe you could eliminate these functions altogether.

In your code you use both CMPU and CMPU6502. Is one of these the base
class, and the other a derived class?

CMain::CMain()
{
m_pMemory = new CMemory(this);
m_pMPU = new CMPU(this);
}

If the new CMPU(this) fails to find memory it throws an exception
std::bad_alloc. The m_pMemory won't be destroyed because the destrructor
CMain::~CMain() cannot be called for incompletely initialized objects, which
are objects that have not fully finished the constructor. Thus you have a
potential memory leak. So the easiest fix is to use smart pointers like

CMain::~CMain()
{
delete m_pMemory;
delete m_pMPU;
}

CMemory* CMain::Get_Memory()
{
return m_pMemory;
}

If Get_Memory always returns valid memory, consider returning a reference
instead of a pointer. Same for the next function.

CMPU* CMain::Get_MPU()
{
return m_pMPU;
}

void CMain::Initialize(void)
{

}

void CMain::Terminate(void)
{

}

void CMain::Run(void)
{
m_pMPU->Run();
}

class CMemory
{
public:
CMemory();
virtual ~CMemory();

CMemory(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

unsigned char Get_MemoryRegister(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

What does it mean to copy a CMemory object through the copy constructor or
operator=?

CMemory::CMemory()
{
Initialize();
}

Variable m_pMain is not initialized, not even to NULL. It points to junk
data. Anyway, do it make sense to have a default constructor? Same
question for CMPU.

CMemory::~CMemory()
{
Terminate();
}

CMemory::CMemory(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMemory::Initialize(void)
{
m_pMemoryRegister = new unsigned char [0x10000];
memset(&m_pMemoryRegister[0], 0x00, 0x10000);
}

void CMemory::Terminate(void)
{
delete [] m_pMemoryRegister;
}

void CMemory::Run(void)
{

}

PU_BYTE CMemory::Get_MemoryRegister(void)
{
return m_pMemoryRegister;
}


class CMPU
{
public:
CMPU();
virtual ~CMPU();

CMPU(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

CMPU::CMPU()
{
Initialize();
}

CMPU::~CMPU()
{
Terminate();
}

CMPU::CMPU(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMPU::Initialize(void)
{
m_pMemoryRegister = m_pMain->Get_Memory()->Get_MemoryRegister();
}

Above means CMPU.cpp has to include CMemory.h in order to use the
Get_MemoryRegister function. Consider changing the constructor to
CMPU(unsigned char *), or adding the function Get_MemoryRegister to class
CMain too.

 

[Bryan Parkoff]

Siemel,

Thank you for the answer. I believe that it is almost exact what you
mean. It is my typo for CMPU and CMPU6502 however I choose to use CMPU as
the primary name instead of CMPU6502. CMemory class and CMPU class are
really sub-classes. They are not derived from base class. It is just base
class alone.
I don't add copy constructor and operator = to each Top class and
sub-class because I have no intention to make copies of one object because I
want to initialize only one object at this time. You recommend that copy
constructor and operator= should be private. I agree with you. You
recommend that it should not be provided by compiler, but only user's
writing code.
Can you please provide your sample code of copy constructor and
operator= that are never used under private?

I want CMPU and CMemory sub-classes to be private except constructor and
deconstructor functions (they are not allowed to be private.) Only CMain
class can access CMPU sub-class and CMemory sub-class through pointer
relationship. I can only be able to write "CMain Main; Main.Run();" under
"int main (void)". "Main.Run()" will point and access CMPU::Run() and
CMemory::Run() that they are always private.
The problem is that CMain class can't access private functions through
pointer relationship.

I am not talking about Memory Manager like CMemory sub-class. I hate to
write "allocate memory" and "deallocate memory" in each sub-classes. I
decide to put "allocate memory" and "deallocate memory" in CMemory sub-class
while CMPU sub-class does not have "allocate memory" and "deallocate
memory".
After memory is being allocated in CMemory sub-class, four bytes of
memory address has to be copied from CMemory sub-class to CMPU sub-class.
It would be very slow to load and store data into array through pointer
relationship from CMPU sub-class to CMemory sub-class, but it would be
easier to access four bytes of memory address indirectly inside CMPU
sub-class.
After deallocating memory in CMemory sub-class, four bytes of address
memory in CMPU sub-class will be set to NULL automatically rather than
deallocating memory second time that can lead to memory leak.
Can you please provide the information about smart pointer? Where can I
find website or book that it can help me to learn how to handle smart
pointer?
Try to think five to ten objects. Five to ten objects are sub-classes
that they point to CMain class. They are like printer, modem, disk,
keyboard, display, and etc. They are all private. If there is no way that
all sub-classes are private, it would be a bad idea to place sub-classes
inside CMain class like nested class. What do you think?
You may be correct that "virtual" is not necessary because I have no
intention to derive a new class from base class.
Please advise.

--
Bryan Parkoff

I have created three classes according to my own design. First class
is called CMain. It is the Top Class. Second class and third class are
called CMemory and CMPU. They are the sub-classes.
Two sub-classes have the relationship to communicate back and forth
through this pointer. The pointer is responsible inside Top class for
allocating and deallocating two sub-classes.

Then the copy constructor and operator= of the sub-classes should probably
be private and not implemented to prevent users from copying one CMemory or
CMPU object to another. Alternatively choose the design in the next
paragraph.

On the other hand, if you clone one of these objects, which CMain does the
cloned object point to? This is an important question, because when the
CMain goes out of scope it destroys the CMemory and CMPU objects it owns, so
if two CMemory objects can point to the same CMain, the CMain has to have an
array of pointers to CMemory objects (not just one pointer as in your class

below).

CMemory class is responsible to allocate and deallocate memory while
CMPU class does not. I tell the pointer to copy m_pMemoryRegister's
memory address from CMemory class to CMPU class. It is done once during
the initialization.

Why does CMPU not own the CMemory class? I think I see the reason: 2 CMPU
objects may share the same memory, right?

After the initialization is done, the functionality can perform all
the tasks inside CMPU class rather than depending on CMemory class so it
can gain a big impact of this performance.
I understand that some C programmers prefer to leave all variables
and functions in the global. In fact, it won't be efficiency because all
variables and functions have fixed memory that they are stored in data
segment. They can't perform the tasks into 80x86's L1 Cache.
It is best to store both variables and functions inside class because
they can be very efficiency. They are stored in stack segment. They can
be able to perform the tasks into 80x86's L1 Cache.
Please read my simple code below. Please state your opinion to see
if my programming code for class is a good design. I appreciate that you
can provide what you think is best.


class CMain
{
public:
CMain();
virtual ~CMain();

CMemory* Get_Memory();
CMPU* Get_MPU();

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMemory* m_pMemory;
CMPU6502* m_pMPU;
};

Since CMain owns the CMemory and CMPU objects, consider using a smart
pointer like std::auto_ptr<CMemory>. You get exception safety in the
constructor, as noted below. Note that if your CMain.h header file does not
include CMemory.h and CMPU.h, you must declare the destructor in the header
file as above, and define it in the cpp file with an empty function body.
You should consider declaring the copy constructor and operator= of CMain
private and not implemented.

In all your classes, Initialize and Terminate are helper functions called
from the constructor and destructor, so make them private. Alternatively,
maybe you could eliminate these functions altogether.

In your code you use both CMPU and CMPU6502. Is one of these the base
class, and the other a derived class?

CMain::CMain()
{
m_pMemory = new CMemory(this);
m_pMPU = new CMPU(this);
}

If the new CMPU(this) fails to find memory it throws an exception
std::bad_alloc. The m_pMemory won't be destroyed because the destrructor
CMain::~CMain() cannot be called for incompletely initialized objects, which
are objects that have not fully finished the constructor. Thus you have a
potential memory leak. So the easiest fix is to use smart pointers like
std::auto_ptr<CMemory> as the member variable m_pMemory. You could also use
new (nothrow), try catch-blocks, etc.

CMain::~CMain()
{
delete m_pMemory;
delete m_pMPU;
}

CMemory* CMain::Get_Memory()
{
return m_pMemory;
}

If Get_Memory always returns valid memory, consider returning a reference
instead of a pointer. Same for the next function.

CMPU* CMain::Get_MPU()
{
return m_pMPU;
}

void CMain::Initialize(void)
{

}

void CMain::Terminate(void)
{

}

void CMain::Run(void)
{
m_pMPU->Run();
}

class CMemory
{
public:
CMemory();
virtual ~CMemory();

CMemory(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

unsigned char Get_MemoryRegister(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

What does it mean to copy a CMemory object through the copy constructor or
operator=?

CMemory::CMemory()
{
Initialize();
}

Variable m_pMain is not initialized, not even to NULL. It points to junk
data. Anyway, do it make sense to have a default constructor? Same
question for CMPU.

CMemory::~CMemory()
{
Terminate();
}

CMemory::CMemory(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMemory::Initialize(void)
{
m_pMemoryRegister = new unsigned char [0x10000];
memset(&m_pMemoryRegister[0], 0x00, 0x10000);
}

void CMemory::Terminate(void)
{
delete [] m_pMemoryRegister;
}

void CMemory::Run(void)
{

}

PU_BYTE CMemory::Get_MemoryRegister(void)
{
return m_pMemoryRegister;
}


class CMPU
{
public:
CMPU();
virtual ~CMPU();

CMPU(CMain* pMain);

void Initialize(void);
void Terminate(void);
void Run(void);

private:
CMain* m_pMain;
unsigned char m_pMemoryRegister;
};

CMPU::CMPU()
{
Initialize();
}

CMPU::~CMPU()
{
Terminate();
}

CMPU::CMPU(CMain* pMain)
{
m_pMain = pMain;
Initialize();
}

void CMPU::Initialize(void)
{
m_pMemoryRegister = m_pMain->Get_Memory()->Get_MemoryRegister();
}

Above means CMPU.cpp has to include CMemory.h in order to use the
Get_MemoryRegister function. Consider changing the constructor to
CMPU(unsigned char *), or adding the function Get_MemoryRegister to class
CMain too.

 

[Steven T. Hatton]

Siemel,

Thank you for the answer. I believe that it is almost exact what you
mean. It is my typo for CMPU and CMPU6502 however I choose to use CMPU as
the primary name instead of CMPU6502. CMemory class and CMPU class are
really sub-classes. They are not derived from base class. It is just
base class alone.

Perhaps this should be obvious to any qualified C++ programmer, but it's not
obvious to me. What are you actually trying to achieve? When I first
looked at your code, I realized that I was not about to compile it and try
it on my system. I had visions of some kind of recursive consumption of
memory without any protection from the OS. But that may just be
paranoia. .

I believe your goal is to somehow keep your resources in the CPU cache, and
micromanage where your program puts data, or something similar. Is this
correct?

 

[Siemel Naran]

Thank you for the answer. I believe that it is almost exact what you
mean. It is my typo for CMPU and CMPU6502 however I choose to use CMPU as
the primary name instead of CMPU6502. CMemory class and CMPU class are
really sub-classes. They are not derived from base class. It is just base
class alone.

BTW, the term sub-class means derived class. As I gathered from your
original post and this one, I think you mean contained class?

I don't add copy constructor and operator = to each Top class and
sub-class because I have no intention to make copies of one object because I
want to initialize only one object at this time. You recommend that copy
constructor and operator= should be private. I agree with you. You
recommend that it should not be provided by compiler, but only user's
writing code.
Can you please provide your sample code of copy constructor and
operator= that are never used under private?

For example,

class CMPU
{
public:
...
private:
CMPU(const CMPU&); // not implemented
CMPU& operator=(const CMPU&); // not implemented
};

If anyone tries to call these functions they get an error that function is
private and not accessible. If you want to get more fancy

class notcopyable
{
public:
notcopyable() { }
private:
notcopyable(const notcopyable&); // not implemented
notcopyable& operator=(const notcopyable&); // not implemented
};

and derived your class CMPU from notcopyable

class CMPU : private notcopyable { ... };

I believe boost has a similar class.

I want CMPU and CMemory sub-classes to be private except constructor and
deconstructor functions (they are not allowed to be private.) Only CMain
class can access CMPU sub-class and CMemory sub-class through pointer
relationship. I can only be able to write "CMain Main; Main.Run();" under
"int main (void)". "Main.Run()" will point and access CMPU::Run() and
CMemory::Run() that they are always private.
The problem is that CMain class can't access private functions through
pointer relationship.

You could use friends. On the other hand, if outside users don't even know
about the CMPU and CMemory objects inside CMain, and they can't get
references to the CMPU etc objects inside CMain, what does it matter if the
functions are public? In fact you could declare class CMPU inside
CMain.cpp, so that users who include CMain.h don't even see these classes at
all.

I am not talking about Memory Manager like CMemory sub-class. I hate to
write "allocate memory" and "deallocate memory" in each sub-classes. I
decide to put "allocate memory" and "deallocate memory" in CMemory sub-class
while CMPU sub-class does not have "allocate memory" and "deallocate
memory".

Is one CMemory object shared by several CMPU objects? If no, then it makes
sense to me to put everything into one class. You could for instance CMPU
contain an instance of a CMemory.

Can you please provide the information about smart pointer? Where can I
find website or book that it can help me to learn how to handle smart
pointer?

Search the web for "smart pointer", "auto_ptr", "shared_ptr", "counted_ptr".

 

[Bryan Parkoff]

Siemel,

Is one CMemory object shared by several CMPU objects? If no, then it makes
sense to me to put everything into one class. You could for instance CMPU
contain an instance of a CMemory.

Yes, two or more CMPU objects point to the same memory address that
CMemory already allocated.

can

I will research to see smart pointer.

Do you understand that printer class, modem class, display class,
keyboard class, and other classes are the only base class that they are
linked to CMain class. They are all private except CMain. Do you suggest
that they should be inside CMain class such as nested class (It is bad
idea.) Or...Should I use "friends" inside CMain class that it can perform
the private functions from printer class, modem class, etc. It makes sense
that friends is useful. Why do you think that printer class, modem class,
etc should be derived from CMain? I understand that sub-class term refer
derived class however variables and functions should notRC]inherited from
CMain class. Please advise.

Bryan Parkoff

 

[Siemel Naran]

Do you understand that printer class, modem class, display class,
keyboard class, and other classes are the only base class that they are
linked to CMain class. They are all private except CMain. Do you suggest
that they should be inside CMain class such as nested class (It is bad
idea.) Or...Should I use "friends" inside CMain class that it can perform
the private functions from printer class, modem class, etc. It makes sense
that friends is useful. Why do you think that printer class, modem class,
etc should be derived from CMain? I understand that sub-class term refer
derived class however variables and functions should notRC]inherited from
CMain class. Please advise.

Use either nested classes or non-nested classes as you see fit. Neither
design is inherently better than the other, but you can't forward declare
nested classes, and they automatically inherit all typedefs/classes of the
parent class which may or may not be a good thing. Sorry if you
misunderstood, I don't think printer etc should derive from CMain, but that
CMain should contain an instance of each of these objects as private data
members.