A Copy of Pointer Is Dangerous?

Discussion in 'C++' started by Immortal_Nephi@hotmail.com, Sep 9, 2008.

  1. Guest

    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
     
    , Sep 9, 2008
    #1
    1. Advertising

  2. Guest

    On Sep 8, 6:45 pm, wrote:

    > 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::eek: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:

    vector<unsigned char> ram(0x1000);

    >
    > 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
     
    , Sep 9, 2008
    #2
    1. Advertising

  3. schrieb:
    >> 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.

    --
    Thomas
     
    Thomas J. Gritzan, Sep 9, 2008
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    30
    Views:
    1,211
  2. Alex
    Replies:
    2
    Views:
    1,291
  3. Replies:
    26
    Views:
    2,181
    Roland Pibinger
    Sep 1, 2006
  4. Replies:
    11
    Views:
    1,042
    Nindi
    Nov 14, 2006
  5. pereges
    Replies:
    7
    Views:
    464
    pereges
    Jun 1, 2008
Loading...

Share This Page