开发者

C++ Custom "List" implementation doesn't work

I tried to create a list container with similar access of elements in c++ as in C# I'm totally lost now because my main method first printed weird numbers.

The RList class should be like:

RList<ClassName or Primitive> VariableName;

VariableName.AddData(Class or Primitive);

VariableName[IndexOfElement] get the element

VariableName.RemoveAt(IndexOfElement) remove element

Can you tell me where I went totally wrong?

int main()
{

RList<int> Numbers;

    Numbers.AddData(5);
    Numbers.AddData(100);
    Numbers.AddData(1500);

for (unsigned int x = 0; x < Numbers.GetLength(); x++)
{

    cout << Numbers[0] << endl;
}



cin.get();

return 0;
}

Here is the Header file. I read that you have to put everything in header if you work with template.

#ifndef RList_H
#define RList_H

#include <new>


template <class T> class RList
{

 private:
    unsigned int m_Length;
    T* ListObject;

    void AllocateNew(T obj);
    void RemoveIndex(unsigned int N);

public:
    RList();
    ~RList();
    void AddData(T obj);
    void RemoveAt(unsigned int N);
    unsigned int Get开发者_开发百科Length() { return m_Length; }
    T operator[](unsigned int N){if (N < m_Length && N >= 0) {return (ListObject[N]);} return NULL; }


};

template <class T>
RList<T>::RList()
{

this->m_Length = 0;


}

template <class T>
RList<T>::~RList()
{

delete[] this->ListObject;


}

template <class T>
void RList<T>::AddData(T obj)
{
this->AllocateNew(obj);
this->m_Length++;

}

template <class T>
void RList<T>::RemoveAt(unsigned int N)
{
if( N < this->m_Length && N >= 0)
{
    if ((this->m_Length - 1) > 0)
    {
            this->RemoveIndex(N);
            this->m_Length--;
    }

    else
    {

    throw "Can't erase last index!";



    }


}



}

template <class T>
void RList<T>::AllocateNew(T obj)
{


if (this->m_Length == 0)
{
    this->ListObject[0] = obj;

}

else
{

T* NewListObject = new T [this->m_Length + 1];

for (unsigned int x = 0; x < this->m_Length; x++)
{
    NewListObject[x] = this->ListObject[x];

}

NewListObject[this->m_Length] = obj;

delete [] ListObject;
this->ListObject = NewListObject;

delete [] NewListObject;

}

}


template <class T>
void RList<T>::RemoveIndex(unsigned int N)
{
T* NewListObject = new T [this->m_Length - 1];
for (int x = 0; x < this->m_Length -1; x++)
{
    if (x != N)
    {

        NewListObject[x] = this->ListObject[x];

    }


}
delete [] ListObject;
this->ListObject = NewListObject;


}
#endif // RList_H


Lots of problems:

  1. Constructors should initialize all members
  2. Rule of three not implemented (on owned pointers).
  3. Horrible spacing (you need to format that code better).
  4. All your array memory allocation is wrong.

Allocate:

template <class T>
void RList<T>::AllocateNew(T obj)
{
    if (this->m_Length == 0)
    {
        // This will not work as you have not allocated the area for ListObjects.
        // I don't think this is a special case. You should have allocated a zero
        // length array in the constructor then then else part would have worked
        // like normal when adding the first element.
        this->ListObject[0] = obj;
    }
    else
    {
        // OK good start
        T* NewListObject = new T [this->m_Length + 1];

        // Rather than do this manually there is std::copy
        for (unsigned int x = 0; x < this->m_Length; x++)
        {
            NewListObject[x] = this->ListObject[x];
        }
        NewListObject[this->m_Length] = obj;

       // Though unlikely there is a posability of an exception from a destructor.
       // So rather than call delete on a member you should swap the member and the
       // temporary. Then when the object is a good state you can delete the old one.
       delete [] ListObject;
       this->ListObject = NewListObject;

       // Definately do NOT do this.
       // as you have just stored this pointer into ListObject.
       // ListObject is now pointing at free'ed memory.
       delete [] NewListObject;    

       // So I would have done (for the last section
       //   std::swap(this->ListObject, NewListObject);
       //   ++this->m_Length;
       //   // now we delete the old data
       //   delete [] NewListObject; // (remember we swapped above)
    }
}

RemoveIndex

template <class T>
void RList<T>::RemoveIndex(unsigned int N)
{
    T* NewListObject = new T [this->m_Length - 1];
    for (int x = 0; x < this->m_Length -1; x++)
    {
        if (x != N)
        {
            // You need to compensate for the fact that you removed one
            // element (otherwise you have a hole in your new array).
            NewListObject[x] = this->ListObject[x];
        }
    }

    // Same comment as above.
    // Do not call delete on a member.
    // Make sure the object is a good state before doing dangerous stuff.
    delete [] ListObject;
    this->ListObject = NewListObject;
}


if (this->m_Length == 0)
{
    this->ListObject[0] = obj;
}

You have to allocate ListObject before you can do this.

Note: there are many problems with your implementation, you should post it to codereview, or check a book to see how to implement a proper vector.


You've got a lot of problems, but this one will surely crash your program:

(at the end of AllocateNew):

this->ListObject = NewListObject;
delete [] NewListObject;

Now, this->ListObject is pointing to memory that's been freed.


Can you tell me where I went totally wrong?

You're reinventing the STL vector class. You can write a thin-wrapper around it to provide the API you want, but it's probably easier to use the the class as-is. Your example would look like this:

#include <iostream>
#include <vector>

using namespace std;

int main( )
{
  vector< int > Numbers;

  Numbers.push_back( 5 );
  Numbers.push_back( 100 );
  Numbers.push_back( 1500 );

  for ( unsigned int x = 0; x < Numbers.size( ); x++ )
  {
    cout << Numbers[x] << endl;
  }

  cin.get();

  return 0;
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜