0

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.

Kelly Elton
  • 4,373
  • 10
  • 53
  • 97
  • 2
    "It doesn't seem to work" isn't enough information. Can you please explain what you did, what result you expected to get and what you actually got? Thanks. – moonshadow Sep 14 '10 at 10:54
  • 1
    may I ask what "bells and whistles" you are trying to avoid to necessitate this class? I still think you will be more productive to just use `std::vector`. – tenfour Sep 14 '10 at 11:02
  • @tenfour I hate to be a dick, but these are the types of unproductive comments I was hoping to avoid. That's why I said "so without going into detail on why I'm writing this class" because it isn't important. I have a need for a custom class, so I'm creating it. I'm not a new to this by any means, I learned c++ through a reference guide without the internet. If I needed to use the Vector class, I would use the vector class. If you post something, just post something productive. That's all I'm saying. – Kelly Elton Sep 14 '10 at 11:06
  • 1
    There are some helpful answers, so here's another unproductive comment: instead of having a custom class, because you didn't want to start from vector, you now have a lot of issues. See http://stackoverflow.com/questions/3563591/how-should-a-size-limited-stl-like-container-be-implemented/3564383#3564383 as an example. – stefaanv Sep 14 '10 at 11:18
  • @stefaanv All tenfour's comment did was undermined my intelligence. It's more of an insult than a help. As I specified in my question, I am fully aware of the vector class. I have a specific need. For my application I require a custom built way to handle my arrays. I DO NOT want to use the vector class. You are also pointing me in the same direction, and that actually happens to be an overkill way to do what I need to do. You were right though, it was unproductive. – Kelly Elton Sep 14 '10 at 11:27
  • I'd like to say that I really appreciate about 50% of people on this site, but I swear the other 50% can't read. – Kelly Elton Sep 14 '10 at 11:30
  • 2
    Another unproductive comment: `_Size`, `_MaxSize` and `_Count` are reserved symbols. It's best to avoid beginning any name with `_`. And I'm with the others: I don't know what "bells and whistles" you're trying to avoid, but your class is larger than `std::vector`, uses memory less efficiently, imposes more conditions on the object type, and leaks memory, in addition to the bugs you're asking about. – Mike Seymour Sep 14 '10 at 11:38
  • @mike seymour First of all the std::Vector class has 850 lines to it. That clears up everything except for your concept that I have memory leaks? It doesn't work yet,and it isn't finished, of course it has problems. But since they are so obvious to you, please, point them out. – Kelly Elton Sep 14 '10 at 11:43
  • 2
    @kelton52: what do the number of code lines have to do with anything? It's more compact (I've no idea why you have three different sizes; `vector` just has `size` and `capacity`), more efficient at runtime (since it doesn't do your bizarre double-reallocation dance, and doesn't allocate twice as much memory as it lets you use), doesn't require objects to be default-constructible (by separating memory allocation from object creation), and has a destructor to free memory (as well as a copy constructor and assignment operator to make that work). Also, it's been thoroughly debugged already. – Mike Seymour Sep 14 '10 at 11:50
  • 1
    @kelton52: I'm with tenfour, stefaanv, and Mike on this. _If I needed a class that's similar to `std::vector`, but differs in some aspects, I'd at least use `std::vector` underneath._ `std::vector` has been very thoroughly designed, reviewed, and debugged. It uses features and optimizations which you, judging from the kind of errors your code shows, might not even be able to appreciate. Honestly, what are you trying to prove here? After a decade of STL experience I was able to write my first vector-like container from scratch on a Sunday afternoon only to find a bug in it several months later. – sbi Sep 14 '10 at 13:19
  • 1
    If I meant to undermine your competence, I would have said "use std::vector" as an answer. I'm sure i'm not the only one curious about your reasons. – tenfour Sep 14 '10 at 14:38
  • You're all missing the point. None of you are helping to debug my code. Your saying I should use the vector class. I didn't ask that. It's that simple. If you can't help me debug my code, then don't comment. Is it really that difficult to understand. All I want is help debugging my code. Do you understand that? I don't care what you think, or how experienced you think you are, or how bad you think this class is. It should be blatantly obvious, I guess, unless again you can't read. Unless you're just skimming over questions and posting obnoxious answers. – Kelly Elton Sep 14 '10 at 20:46
  • And I want to iterate on this again. It's not done, it's not finished, it's not complete, there are idioms in the code due to me trying to debug the thing. If I had a complete class, let me ask you, why would I bring it here? I appreciate the people below that took the time to read my question, and actually try and solve the problem. You other people are just wasting your time and looking a bit ridiculous in the process. – Kelly Elton Sep 14 '10 at 20:57
  • @kelton52: I'm sure all the commenters read and understood your question perfectly well. However, this site isn't a personal debugging service. The answers are intended for anyone facing a similar problem; as such, people will give whatever advice they think is useful. In virtually every case, problems encountered when implementing a dynamic array are best solved by using the existing solution, taking advantage of decades of careful refinement, and devoting your efforts to solving new problems. You have your answers; if you find further advice obnoxious then feel free to ignore it. – Mike Seymour Sep 15 '10 at 00:35

4 Answers4

1

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
  • 476,176
  • 80
  • 629
  • 1,111
  • 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. – Kelly Elton Sep 14 '10 at 11:09
1
   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
  • 86,889
  • 7
  • 82
  • 122
  • 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. – Kelly Elton Sep 14 '10 at 11:17
  • _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. – Kelly Elton Sep 14 '10 at 11:19
  • haha, yeah...Sometimes that slips my mind when I switch from a single line after the if. Doesn't fix the problem though. – Kelly Elton Sep 14 '10 at 11:20
0

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
  • 8,521
  • 5
  • 40
  • 64
0

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
  • 40,819
  • 6
  • 57
  • 90
  • 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. – Kelly Elton Sep 14 '10 at 20:48