1

As an exercise, I'm trying to create a class myArray that acts as a simplified array class. Here is my header:

#ifndef myArray_h
#define myArray_h

typedef double ARRAY_ELEMENT_TYPE;

class myArray {

public:
//--constructors
    myArray(int initMax);
    // post: Allocate memory during pass by value

    myArray(const myArray & source);
    // post: Dynamically allocate memory during pass by value

//--destructor
    ~myArray();
    // post: Memory allocated for my_data is deallocated.

//--modifier
    void set(int subscript, ARRAY_ELEMENT_TYPE value);
    // post: x[subscript] = value when subscript is in range.
    //       If not, an error message is displayed.

//--accessor
    ARRAY_ELEMENT_TYPE sub(int subscript) const;
    // post: x[subscript] is returned when subscript is in range.
    //       If not, display an error message and return [0].

private:
ARRAY_ELEMENT_TYPE* my_data;
int my_capacity;
};
#endif

Here is my implementation:

#include "myArray.h"
#include <iostream>
#include <cstring>

using namespace std;

typedef double ARRAY_ELEMENT_TYPE;

//--constructors
myArray::myArray(int initMax)
{
my_capacity = initMax;
}

myArray::myArray(const myArray & source)
{
int i;
my_data = new ARRAY_ELEMENT_TYPE[source.my_capacity];

for(i=0; i < my_capacity; i++)
    my_data[i] = source.sub(i);
}

//--destructor
myArray::~myArray()
{
delete [ ] my_data;
}

//--modifier
void myArray::set(int subscript, ARRAY_ELEMENT_TYPE value)
{
if(subscript > my_capacity - 1)
{
    cout << "**Error: subscript " << subscript << " not in range 0.." << my_capacity-1 << ". The array is unchanged." << endl;
}
else
    my_data[subscript] = value;
}

//--accessor
ARRAY_ELEMENT_TYPE myArray::sub(int subscript) const
{
if(subscript >= my_capacity)
{
    cout << "**Error: subscript " << subscript << " not in range 0.." << my_capacity-1 << ". Returning first element." << endl;
    cout << my_data[0];
}
else
{
    return my_data[subscript];
}
}

And I'm using this as a test driver:

#include <iostream>
using namespace std;
typedef double ARRAY_ELEMENT_TYPE;
#include "myArray.h"

void show (const myArray & arrayCopy, int n)
{
for(int j = 0; j < n; j++)
    cout << arrayCopy.sub(j) << endl;
}

int main()
{
int n = 6;
myArray a(6);
a.set(0, 1.1);
a.set(1, 2.2);
a.set(2, 3.3);
a.set(3, 4.4);
a.set(4, 5.5);
a.set(5, 6.6);
show(a, n);
cout << a.sub(11) << endl;
a.set(-1, -1.1);

return 0;
}

The problem is that when I run this, I get nothing for a bit, then the "Press any key to continue..." prompt. What's going wrong?

spartanhooah
  • 183
  • 4
  • 15
  • 1
    Don't forget the [Rule of Three](http://stackoverflow.com/questions/4172722). Two out of three ain't bad, but you're missing a copy-assignment operator. – Mike Seymour Aug 16 '13 at 18:18

3 Answers3

1

The myArray constructor doesn't allocate memory for my_data. The first time you call set, it attempts to write to an uninitialised pointer. This results in undefined behaviour but a crash is likely.

You should change the constructor to

myArray::myArray(int initMax)
{
    my_capacity = initMax;
    my_data = new ARRAY_ELEMENT_TYPE[my_capacity];
}

There are a couple of other issues with the code you could also consider

In 'set', the test

if(subscript > my_capacity - 1)

should be

if(subscript < 0 || subscript > my_capacity - 1)

Or you could change the subscript argument to have type unsigned int.

In sub, the line cout << my_data[0]; should presumably be return my_data[0];

simonc
  • 41,632
  • 12
  • 85
  • 103
  • That solved most of it. Now I get a warning that not all return paths in `sub` return a value, but I can't figure out what would fall outside of `if(subscript >= my_capacity)` and `else`. I get a crash on the last call of `a.set(-1, -1.1)`, with the output saying `1.1-1.#IND`, and a heap corruption debug error. – spartanhooah Aug 16 '13 at 18:19
  • I've updated my answer with notes of what's causing each of these issues – simonc Aug 16 '13 at 18:22
  • I'd missed your last line, which got rid of the #IND problem. Thanks for your help (since I can't vote up yet). – spartanhooah Aug 16 '13 at 19:10
1
myArray::myArray(int initMax)
{
my_capacity = initMax;
my_data = new ARRAY_ELEMENT_TYPE[my_capacity]; //You missed this
}
P0W
  • 46,614
  • 9
  • 72
  • 119
0

In addition to missing your allocation in your current implementation, you are also dynamically allocating memory. A simple array type does not need to be allocated on the heap. The std::array collection does exactly what you are looking to do. I would urge you to look at its implementation for an example (if this is just an academic exercise). If this is for a production codebase, use what is already written and tested.

http://en.cppreference.com/w/cpp/container/array

Zac Howland
  • 15,777
  • 1
  • 26
  • 42
  • `std::array` is statically sized. Until we get dynamically-sized arrays and `std::dynarray` next year, we need dynamic allocation if the size isn't known at compile time. (And the question does state that this is an exercise, so "use `vector`" isn't a helpful answer either). – Mike Seymour Aug 16 '13 at 21:40
  • I wasn't stating to use `std::array` or `std::vector` (unless it is for a production codebase). I was rather explicit that he should look at the implementation of `std::array` to see how it is done. His current implementation creates an un-resizeable dynamically allocated array (in other words, he knows how big it is at compile time, but is allocating it on the heap anyway). – Zac Howland Aug 19 '13 at 13:51
  • No, the current implementation takes the size as a run-time constructor parameter, not a compile-time template parameter as `array` does. Until next year, you need dynamic allocation to deal with that. (In the example, the size is a constant; but there's no indication that the class should be restricted to compile-time values). – Mike Seymour Aug 19 '13 at 13:53