A Copy of Pointer Is Dangerous?

[Immortal_Nephi]

Sometimes, unsigned char array is located in the file scope. You
define A Object and B Object. A Object and B Object need to share
unsigned char array. They are very different object. They are like
two chips.
A Object modifies data in the unsigned char array. Then, B Object is
in turn to modify data in the unsigned char array. Static data member
is not the answer. Unsigned char array in the file scope is a bad
idea because other functions or classes might modify unsigned char
array accidently. The only way is to guard it inside main() function.
You need to define two pointers. Two pointers are used to share
unsigned char array. You have to be careful when you write your code
in the correct sequence.
Sometimes, you may want to copy data member from A Object to B
Object. Friend class is the answer. Please say if Update() function
or Update2() function is better. Update2() function may have
additional overhead because "this pointer" reads memory address
indirection before indirection memory address is read again to another
indirection memory address and then accesses data member. Update()
function uses reference to A Object.
Compare Run() function in both A Object and B Object.
Place NULL in unsigned char array in both ~A() function and ~B()
function are unnecessary.
Please state your opinion what you think my good design C++ code.
Here is an example of my code below.

class B;

class A
{
public:
A( unsigned char *RAM )
{
this->RAM = RAM;
this->a = 'A';
this->b = 'B';
this->c = 'C';
}

~A() { this->RAM = 0; }

void Set_B_Ptr(B &b)
{
this->B_Ptr = &b;
}

void Run()
{
this->RAM[0] = this->a;
this->RAM[1] = this->b;
this->RAM[2] = this->c;
}

private:
friend class B;
unsigned char* RAM;
unsigned char a;
unsigned char b;
unsigned char c;
B *B_Ptr;
};

class B
{
public:
B( unsigned char *RAM )
{
this->RAM = RAM;
this->d = 'D';
this->e = 'E';
this->f = 'F';
}

~B() { this->RAM = 0; }

void Set_A_Ptr(A &a)
{
this->A_Ptr = &a;
}

void Run()
{
this->RAM[3] = this->d;
this->RAM[4] = this->e;
this->RAM[5] = this->f;
}

void Update(A &a)
{
this->a = a.a;
this->b = a.b;
this->c = a.c;
}

void Update2()
{
this->a = this->A_Ptr->a;
this->b = this->A_Ptr->b;
this->c = this->A_Ptr->c;
}

private:
friend class A;
unsigned char* RAM;
unsigned char a;
unsigned char b;
unsigned char c;
unsigned char d;
unsigned char e;
unsigned char f;
A *A_Ptr;
};


int main()
{
unsigned char* RAM = new unsigned char [0x1000];

for (int zero = 0; zero < 0x1000; zero++)
RAM[zero] = 0;

A a(RAM);
B b(RAM);

a.Set_B_Ptr( b );
b.Set_A_Ptr( a );

a.Run();
b.Run();

b.Update( a );
b.Update2();

delete [] RAM;

return 0;
}

Nephi

 

[acehreli]

A Object modifies data in the unsigned char array. Then, B Object is
in turn to modify data in the unsigned char array. Static data member
is not the answer. Unsigned char array in the file scope is a bad
idea because other functions or classes might modify unsigned char
array accidently. The only way is to guard it inside main() function.

Since introducing the shared data in main is simply natural, there is
no reason to put the data in file scope.

Sometimes, you may want to copy data member from A Object to B
Object. Friend class is the answer.

You can also use A:perator=. That way, you don't need to be a friend
of A.

Please say if Update() function
or Update2() function is better.

Could you have an A member in B, instead of having members of A? Then
this would be better:

Update3(const A & a)
{
a_ = a;
// Sorry, but I can't stand the 'this->' prefix
// The quieter postfix '_' is much better.
}

Update2() function may have
additional overhead because "this pointer" reads memory address
indirection before indirection memory address is read again to another
indirection memory address and then accesses data member.

That's true that Update2() has extra indirection(s). It's probably
irrelevant tothe problem at hand.

Update()
function uses reference to A Object.

Oh... Then they are probably equally expensive, because the reference
parameter is implemented as a pointer behind the scenes anyway.

Compare Run() function in both A Object and B Object.

I see two functions that modify distinct parts of a third object.

Place NULL in unsigned char array in both ~A() function and ~B()
function are unnecessary.

Correct. Changing the members of a dead object shouldn't matter unless
you access that dead object after its death, which is undefined
behavior anyway.

Please state your opinion what you think my good design C++ code.

It is not clear why A and B need each other.

class A
{
public:
A( unsigned char *RAM )
{
this->RAM = RAM;
this->a = 'A';
this->b = 'B';
this->c = 'C';
}

Always use the initialization list unless there is a reason not to.
Also, B_Ptr is left uninitialized.

~A() { this->RAM = 0; }

Completely unnecessary and possibly hides a bug somewhere else by
changing the undefinedness of the undefined behavior, if in fact there
is one.

int main()
{
unsigned char* RAM = new unsigned char [0x1000];

You must use a type that knows how to cleanup after itself. If there
is an exception in the program, you may not reach the delete[] line.

Reserve all capitals to macro names. So, how about:

for (int zero = 0; zero < 0x1000; zero++)
RAM[zero] = 0;

'zero' is a very misleading name. Prefer the idiomatic 'i' instead.
Also, use the curly braces even if there is only one line.

Additionally, prefer pre-increment unless you really mean "increment
this object but use its old value."

for (int i = 0; i != 0x1000; ++i) {
RAM = '\0';
}

Still, the entire loop is unnecessary because both new[] and
vector<unsigned char> would initialize the contents to '\0' anyway.

Ali

 

[Thomas J. Gritzan]

unsigned char* RAM = new unsigned char [0x1000];

[...]
for (int i = 0; i != 0x1000; ++i) {
RAM = '\0';
}

Still, the entire loop is unnecessary because both new[] and
vector<unsigned char> would initialize the contents to '\0' anyway.

Wrong. new[] only initializes the elements of the array to 0 (or '\0')
if you use the form with parenthesis:

unsigned char* Ram = new unsigned char [0x1000]();

But that doesn't work on all compilers.

So better to use std::vector.