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:
- Constructors should initialize all members
- Rule of three not implemented (on owned pointers).
- Horrible spacing (you need to format that code better).
- 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;
}
精彩评论