2

I am trying to get this code to compile in GNU C++ Compiler (g++), but it doesn't seem to be working. I've used Vis Studio and Code::Blocks and it works fine. I am aware that compilers differ in certain ways and was wondering if anyone could help me find my error.

#include <iostream>
using namespace std;

template <class T>
class Array
{
    private:
        T *m_array;
        int m_size;
    public:
        Array();
        Array(Array& other);
        Array(int size);
        ~Array();
        void setValue(int index, T val);
        T getValue(int index);
        int getSize();
        Array &operator=(Array &other);
        Array &operator+(Array &other);
        Array &operator+(T val);
        inline friend ostream &operator<<(ostream &other, Array<T> arr)
        {
            for (int i = 0; i < arr.getSize(); i++)
            {
                other << arr.getValue(i) << " ";
            }
        }
};

template<class T>
Array<T>::Array()
{
    m_array =  NULL;
    m_size = 0;
}
template<class T>
Array<T>::Array(int size)
{
    m_size = size;
    m_array = new T[size];
}
template<class T>
Array<T>::Array(Array& other)
{
    *this = other;
}
template<class T>
Array<T>::~Array()
{
    delete[] m_array;
}
template<class T>
void Array<T>::setValue( int index, T val )
{
    m_array[index] = val;
}
template<class T>
T Array<T>::getValue( int index )
{
    return m_array[index];
}
template<class T>
Array<T> &Array<T>::operator=( Array& other )
{
    if (m_array != NULL)
        delete[] m_array;

    m_size = other.getSize();

    m_array = new T[m_size];

    for (int i = 0; i < other.getSize(); i++)
    {
        m_array[i] = other.getValue(i);
    }

    return *this;
}
template<class T>
Array<T> &Array<T>::operator+( Array &other )
{
    for (int i = 0; i < m_size; i++)
    {
        m_array[i] += other.getValue(i);
    }

    return *this;
}
template<class T>
Array<T> &Array<T>::operator+( T val )
{
    for (int i = 0; i < m_size; i++)
    {
        m_array[i] += val;
    }

    return *this;
}
template<class T>
int Array<T>::getSize()
{
    return m_size;
}
sosolal
  • 17
  • 6

3 Answers3

1

1) You should really learn about const-correctness

2) This code looks suspicious

template<class T>
Array<T> &Array<T>::operator+( Array &other )
{
    for (int i = 0; i < m_size; i++)
    {
        m_array[i] += other.getValue(i);
    }

    return *this;
}

What if the other Array has less elements? You'll have undefined behavior (which can include segmentation fault)

3) Why did you use m_array[i] += other.getValue(i);? Because m_array is private? Remember, access is defined at class level, not object level, so m_array[i] = other.m_arry[i] would work as well.

4) I suggest that you should read a good C++ book

5) the exact reason of the segfault can be determined only when you post the code that uses your array class.

Community
  • 1
  • 1
Armen Tsirunyan
  • 130,161
  • 59
  • 324
  • 434
  • Yes, I agree that should be fixed. But I believe that the error is resulting from something happening in the << overloaded operator. As I use bits of test code to see what's happening right before the crash. – user1821626 Nov 13 '12 at 18:07
  • @user1821626: That would be a compiler error. You're saying that you're getting a segmentation fautl – Armen Tsirunyan Nov 13 '12 at 18:07
1

Turn on your warnings:

g++ -std=c++0x -pedantic -Wall -Werror -g    x.cc   -o x
cc1plus: warnings being treated as errors
x.cc: In function ‘std::ostream& operator<<(std::ostream&, Array<T>)’:
x.cc:27: error: no return statement in function returning non-void

And the incorrect function is:

inline friend ostream &operator<<(ostream &other, Array<T> arr)
Robᵩ
  • 163,533
  • 20
  • 239
  • 308
1

I see two problems:

  1. Use const &, otherwise your array will be copied:

    inline friend ostream &operator<<(ostream &other, const Array &arr)

  2. Don't use the assignment operator in a constructor without initializing pointers:

    Array::Array(Array& other) { *this = other; }

this should be at least:

Array<T>::Array(const Array& other)
    : m_array(0)
{
    *this = other;
}

And I guess this is where it crashes:

template<class T>
Array<T> &Array<T>::operator=( Array& other )
{
    if (m_array != NULL)
        delete[] m_array; // In copy constructor, deletes uninitialized pointer!

    m_size = other.getSize();

    m_array = new T[m_size];

    for (int i = 0; i < other.getSize(); i++)
    {
        m_array[i] = other.getValue(i);
    }

    return *this;
}
Olaf Dietsche
  • 72,253
  • 8
  • 102
  • 198