Array Class Printout of the array

Question

I am trying to create a member function which print out the array that I control, but I am running into Seg fault. Any help would be really useful!

Here is my header file, all the code in there work except the last member function.

#include <iostream>
#include <assert.h>

using namespace std;
#ifndef _ARRAY_H
#define _ARRAY_H

template<class T>
class Array{
  private:
    T *a;
    int length;
  public:
    // constructor
    Array (int len){
      length = len;
      a = new T[length];
      for (int i = 0; i < len; i++){
        a[i]=0;
      }
    }
    // destructor
    ~Array()
      {delete[] a;}
    // operator overload
    T& operator [](int i){
      assert (i>=0 && i < length);
      return a[i];
    }

    // operator overload
    Array<T>& operator=(Array<T> &b){
      if (a !=nullptr) delete[] a;
      a = b.a;
      b.a = nullptr;
      length = b.length;
      return *this;
    }

    //get the length of the array
    int arraylength(){
      return length;
    }
//------------------This below is where I am having issue --------//
    //print out the array
    Array<T> printarray(){
      for (int i = 0; i < length; i++){
        cout << a[i];
      }
    }
};
int main();
#endif

This is my main file

#include <iostream>
#include "../include/array.h"

using namespace std;

int main(){

  // initialize array
  Array <int> a(5);
  Array <int> b(5);

  // put stuff into array
  for (int i = 0; i< a.arraylength(); i++){
    a[i] = i;
  }
  // set b = a using operator overload
  b = a;

  // print out the result b array
  for (int i = 0; i < b.arraylength(); i++){
    cout << b[i] << endl;
  }
  a.printarray();
  return 0;
}

Again. Thank you for the help, I am quite new to C++ and mostly self taught.

Don't use using namespace in headers! <assert.h> is called <cassert> in C++. Why do you #include files in your header before the include guard (#ifndef/#define/#endif)? The correct type for sizes of objects in memory and indexes into them is std::size_t (<cstddef>), not int. Look up the "Rule of 3/5/0" and the copy-and-swap idiom. — Swordfish
– Swordfish, Commented Oct 18, 2018 at 7:44

rafix07 · Accepted Answer · 2018-10-18 05:38:58Z

1

In this statement

  b = a;

you have called operator= in which a pointer of a object was set to nullptr, but in printArray you don't check if a is not null, so you are accesing data for null pointer, it is undefined behaviour. Add the condition to check if array is not empty:

void printarray(){
      if (!a) return;  // <- array is empty
      for (int i = 0; i < length; i++){
        cout << a[i];
      }
    }

Secondly, return type of printArray should be void, you don't return any value in this function.

answered Oct 18, 2018 at 5:38

rafix07

21.3k3 gold badges26 silver badges36 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

R Sahu · Accepted Answer · 2018-10-18 14:10:06Z

1

You should fix printarray by changing the return type to void and making it a const member function.

void printarray() const {
   for (int i = 0; i < length; i++){
      cout << a[i];
   }
}

However, that is not the main problem in your code. The main problem is that you are not following the The Rule of Three.

You don't have copy constructor.
You have a copy assignment operator but it is not implemented properly.

The line

b = a;

causes problems downstream that can be fixed by following The Rule of Three.

Here's an implementation of the copy assignment operator function that should work.

// Make the RHS of the operator a const object.
Array<T>& operator=(Array<T> const& b)
{
   // Prevent self assignment.
   // Do the real assignment only when the objects are different.
   if ( this != &b )
   {
      if (a != nullptr)
      {
         delete[] a;
         a = nullptr;
      }

      // This is not appropriate.
      // a = b.a;
      // b.a = nullptr;

      // b needs to be left untouched.
      // Memory needs to be allocated for this->a.
      length = b.length;
      if ( length > 0 )
      {
         a = new T[length];

         // Copy values from b to this.
         for (int i = 0; i < length; ++i )
         {
            a[i] = b.a[i];
         }
      }
   }
   return *this;
}

Please note that you should implement the copy constructor also, and then use the copy swap idiam to implment the assignment operator.

Very relevant: What is the copy-and-swap idiom?

edited Oct 18, 2018 at 14:10

answered Oct 18, 2018 at 5:38

R Sahu

207k15 gold badges168 silver badges291 bronze badges

3 Comments

Andy Lau Over a year ago

Thank you for the extra information! Super helpful!

Swordfish Over a year ago

@RSahu I'd rather use copy-and-swap.

R Sahu Over a year ago

@Swordfish, me too.

Collectives™ on Stack Overflow

Array Class Printout of the array

2 Answers 2

Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related