tags:

views:

102

answers:

4

Alright, so without going into detail on why I'm writing this class, here it is.

template<class aType>
  class nArray
  {
  public:
   aType& operator[](int i)
   {
    return Array[i];
   }
   nArray()
   {
    aType * Array = new aType[0];
    _Size = 0;
    _MaxSize = 0;
    _Count = 0;
   }
   nArray(int Count)
   {
    aType * Array = new aType[Count*2]();
    _Size = Count;
    _MaxSize = Count * 2; 
    _Count = 0;
   }

   int Resize(int newSize)
   {
    aType *temp = new aType[newSize*2];
    for(int i=0;i<_Count;i++)
    {
     temp[i] = Array[i];
    }
    delete[] Array;
    aType * Array = new aType[newSize*2];
    for(int i=0;i<_Count;i++)
    {
     Array[i] = temp[i];
    }
    delete [] temp;
    _Size = newSize;
    _MaxSize = newSize*2;
    return 0;
   }
   int Push_Back(aType Item)
   {
    if(_Count+1 >= _Size)
    {
     Resize(_MaxSize);
    }
    Array[_Count] = Item;
    _Count++;
    return _Count - 1;
   }
   aType GetAt(int Index, int &ret)
   {
    if(Index > _Size-1)
     ret = 1;
     return aType();
    ret = 0;
    return Array[Index];
   }
  private:
   int _Size;
   int _Count;
   int _MaxSize;
   aType * Array;
  };

It is supposed to be a std::Vector type object, without all the bells and whistles. Problem is, it doesn't seem to work.

I basically start by going

nArray<string> ca = nArray<string>(5);
ca.Push_Back("asdf");
ca.Push_Back("asdf2");
int intret = 0;
cout << ca.GetAt(1,intret);

I get an Access Violation Reading Location error and it hits on the line

Array[_Count] = Item 

in the Push_back function.

The problem seems to be that it's not treating the Array object as an array in memory.

I've spent time going through the code step by step, and I don't know what else to say, it's not operating right. I don't know how to word it right. I'm just hoping someone will read my code and point out a stupid mistake I've made, because I'm sure that's all it amounts to. Update So now I changed 3 initializations of Array in nArray(), nArray(int Count), and Resize(int newSize)

    template<class aType>
    class nArray
    {
    public:
        aType& operator[](int i)
        {
            return Array[i];
        }
        nArray()
        {
            Array = new aType[0];
            _Size = 0;
            _MaxSize = 0;
            _Count = 0;
        }
        nArray(int Count)
        {
            Array = new aType[Count*2]();
            _Size = Count;
            _MaxSize = Count * 2; 
            _Count = 0;
        }

        int Resize(int newSize)
        {
            aType *temp = new aType[newSize*2];
            for(int i=0;i<_Count;i++)
            {
                temp[i] = Array[i];
            }
            delete[] Array;
            Array = new aType[newSize*2];
            for(int i=0;i<_Count;i++)
            {
                Array[i] = temp[i];
            }
            delete [] temp;
            _Size = newSize;
            _MaxSize = newSize*2;
            return 0;
        }
        int Push_Back(aType Item)
        {
            if(_Count+1 >= _Size)
            {
                Resize(_MaxSize);
            }
            Array[_Count] = Item;
            _Count++;
            return _Count - 1;
        }
        aType GetAt(int Index, int &ret)
        {
            if(Index > _Size-1)
                ret = 1;
                return aType();
            ret = 0;
            return Array[Index];
        }
    private:
        int _Size;
        int _Count;
        int _MaxSize;
        aType * Array;
    };

This is how my code was before. Anyway, the original problem was the fact that when I try to access a specific element in the array, it just accesses the first element, and it doesn't seem to add elements eather. It doesn't seem to be treating Array as an array.

+1  A: 

You have a number of problems. At a guess, the one causing problems so far is that your default ctor (nArray::nArray()) defines a local variable named Array that it initializes, which leaves nArray::Array uninitialized.

Though you probably haven't seen any symptoms from it (yet), you do have at least one more problem. Names starting with an underscore followed by a capital letter (such as your _Size, _MaxSize, and _Count) are reserved for the implementation -- i.e., you're not allowed to use them.

The logic in your Resize also looks needlessly inefficient (if not outright broken), though given the time maybe it's just my brain not working quite right at this hour of the morning.

Jerry Coffin
Yeah I thought the resize function had problems so I drug it out quit a bit to follow what was going on. I also changed the Array initialization in 3 functions to the aType * Array from just Array, because I thought that would solve my original problem, but I guess this gets rid of my memory issues.
kelton52
A: 

Your array is not initialized by the constructors and resize function (working on local vars instead).

And is there a reason you want to store instances of string and not pointers to string (string *) ?

jv42
Because I want to be able to destroy the strings and pass the array around.
kelton52
+1  A: 
   int Resize(int newSize)
   {
    .
    .
    aType * Array = new aType[newSize*2];

At this point, instead of updating the member variable as you intended, you've actually created a local variable called Array whose value is discarded when you exit from Resize(). Change the line to

    Array = new aType[newSize*2];

The same thing is happening in your constructors, they also need changing accordingly. Moreover, since the default constructor allocates an array, you should set the size members accordingly. You have too many of these: an array needs to keep track of current element count and maximum capacity, however you appear to have three members. What is the purpose of the third? Redundant information is bad, it makes code difficult to read and without a single point of truth it is easier to make mistakes.

With the code in Resize(), you can do better: the second copy is completely redundant.

   int Resize(int newSize)
   {
    aType *temp = new aType[newSize*2];
    for(int i=0;i<_Count;i++)
    {
     temp[i] = Array[i];
    }
    delete[] Array;
    Array = temp;
    _Size = newSize;
    _MaxSize = newSize*2;
    return 0;
   }

Also, in

aType GetAt(int Index, int &ret)
        {
            if(Index > _Size-1)
                ret = 1;
                return aType();
            ret = 0;
            return Array[Index];
        }

you need curly braces around body of the if(), just indentation on its own won't do the trick:

aType GetAt(int Index, int &ret)
        {
            if(Index > _Size-1)
            {
                ret = 1;
                return aType();
            }
            ret = 0;
            return Array[Index];
        }
moonshadow
That's how I had the resize function, but it wasn't working, so I changed it because I wasn't sure how it was copying the class. That's why it is so drawn out.
kelton52
_Size is the current size the array tells the user it has available..._MaxSize is the maximum size before realocatting..._Count is the actual number of elements.
kelton52
haha, yeah...Sometimes that slips my mind when I switch from a single line after the if. Doesn't fix the problem though.
kelton52
A: 

I think the answer after the changes is in moonshadow's reply:

    aType GetAt(int Index, int &ret)
    {
        if(Index > _Size-1)
            ret = 1;
            return aType();
        ret = 0;
        return Array[Index];
    }

This code will always return aType(), the last two lines will never be reached.

You might also want to check what happens if you start out with a default-constructed nArray. (Hint: you call Resize(_MaxSize); but what is the value of _MaxSize in this case?


Edit: This outputs "asdf2" for me as it should be (with the initialization and the braces fixed):

template<class aType>
class nArray
{
public:
    aType& operator[](int i)
    {
        return Array[i];
    }
    nArray()
    {
        Array = new aType[0];
        _Size = 0;
        _MaxSize = 0;
        _Count = 0;
    }
    nArray(int Count)
    {
        Array = new aType[Count*2]();
        _Size = Count;
        _MaxSize = Count * 2;
        _Count = 0;
    }

    int Resize(int newSize)
    {
        aType *temp = new aType[newSize*2];
        for(int i=0;i<_Count;i++)
        {
            temp[i] = Array[i];
        }
        delete[] Array;
        Array = new aType[newSize*2];
        for(int i=0;i<_Count;i++)
        {
            Array[i] = temp[i];
        }
        delete [] temp;
        _Size = newSize;
        _MaxSize = newSize*2;
        return 0;
    }
    int Push_Back(aType Item)
    {
        if(_Count+1 >= _Size)
        {
            Resize(_MaxSize);
        }
        Array[_Count] = Item;
        _Count++;
        return _Count - 1;
    }
    aType GetAt(int Index, int &ret)
    {
        if(Index > _Size-1) {
            ret = 1;
            return aType();
        }
        ret = 0;
        return Array[Index];
    }
private:
    int _Size;
    int _Count;
    int _MaxSize;
    aType * Array;
};

#include <string>
#include <iostream>
using namespace std;

int main()
{
    nArray<string> ca = nArray<string>(5);
    ca.Push_Back("asdf");
    ca.Push_Back("asdf2");
    int intret = 0;
    cout << ca.GetAt(1,intret);
}
UncleBens
It's defininatly not complete. I was a little into designing it and I realized that it's not treating Array as an array. I have already corrected the curly brace problem, but I still have the same result.
kelton52