Problems with self created template matrix class

Question

I'm trying to write code that implemets a template for a matrix with boundary check and additional operators. However, I'm stuck at a SIGGSEGV right now. The matrix is based on a vector class I wrote myself, too:

#ifndef VEKTOR_H
#define VEKTOR_H
#include "Exeptions.h"
template<typename T>
class vektor
{  
    private:
        unsigned int length;
        T* items;

    public:
        inline unsigned int getLength() const{return length;};

        vektor():length(0),items(nullptr)
        {
            //ctor
        }

        virtual ~vektor()
        {
            delete [] items;
        }

        vektor(const vektor& other):length(other.getLength()),items(new T[length])
        {
            for(int i = 0;i<length;i++)
            {
                items[i]=other[i];
            }
        }

        vektor(int len):length(len),items(new T[len])
        {

        }

        vektor<T>& operator=(const vektor& rhs)
        {
            if (this == &rhs) return *this; // handle self assignment
            //assignment operator
            length = rhs.getLength();
            items = new T[length];
            for(int i = 0;i<length;i++)
            {
                items[i]=rhs[i];
            }
            return *this;
        }

        T& operator[](const unsigned int index)const
        {
            if(index >= 0 && index < length)
            {
                return items[index];
            }
            else throw out_of_bounds();
        }
};
#endif // VEKTOR_H

Then the code for the matrix:

#ifndef MATRIX_H
#define MATRIX_H
#include "vektor.h"
template <typename T>
class matrix
{
    private:
        int columns;
        vektor<T>* tabs;

    public:
        inline int getColCount()const{return columns;};
        inline int getRowCount()const{return tabs[0].getLength();};
        inline vektor<T>* getTabs()const{return tabs;};

        matrix():columns(0),tabs(new vektor<T>(0))
        {
            //ctor
        }
        matrix(int columns, int rows):columns(columns),tabs(new vektor<T>[columns])
        {
            for(int i = 0; i< columns;i++)
            {
                tabs[i] = *new vektor<T>(rows);
            }
        }
        virtual ~matrix()
        {
            delete tabs;
        }
        matrix(const matrix& other):rows(other.getColCount()),tabs(new vektor<T>(*other.getTabs()))
        {
            //copy ctor
        }

        matrix<T>& operator=(const matrix& rhs)
        {
            if (this == &rhs) return *this; // handle self assignment
            //assignment operator
            spalten = rhs.getColCount();
            tabs = new vektor<T>(*rhs.getTabs());
            return *this;
        }

        vektor<T>& operator[](unsigned int index)
        {
            return tabs[index];
        }
};

#endif // MATRIX_H

A wrapper class that instantiates a matrix with string to create a menu-like structure:

MenUI::MenUI():selectedTab(3),selectedItem(4),menItems(matrix<string>(3,4))
{
    menItems[0][0] = "File";
    menItems[0][1] = "Edit";
    menItems[0][2] = "View";
    menItems[0][3] = "Search";
    menItems[1][0] = "New";
    menItems[1][1] = "Undo";
    menItems[1][2] = "Perspectives";
    menItems[1][3] = "Find";
    menItems[2][0] = "Open...";
    menItems[2][1] = "Redo";
    menItems[2][2] = "Toolbars";
    menItems[2][3] = "Find in Files";
}

And here the SIGSEGV happens at line tmp = menItems[selectedTab][i];

void MenUI::print()
{
    int offset = 0;
    string tmp;
    for(unsigned int i = 0; i<menItems.getColCount();i++)
    {
        tmp = menItems[i][0];
        if (i == selectedTab) cout  << "|- " << setw(10) << tmp << " -";
        else cout << "|  " << setw(10) << tmp << "  ";
    }
    cout << "|" << endl;
    offset = selectedTab * (10 + 5);
    for(unsigned int i = 1;i<menItems.getRowCount();i++)
    {
        tmp = menItems[selectedTab][i];
        if(i == selectedItem) cout << string(offset-3, ' ') << "|> " << setw(10) << tmp << " <|" << endl;
        else cout << string(offset-3, ' ') << "|  " << setw(10) << tmp << "  |" << endl;
    }
}

As this is the first time I used templates, I'm a bit lost right now. I'm using Code::Blocks IDE with GCC. It would be really great if someone could point me in the right direction. Thanks!

Since you already implemented a vector class, why not use it? For example `vektor> tabs;` instead of `vektor* tabs;`. That's assuming you have a reason for needing your own vector type, instead of `std::vector`. — François Andrieux, Jan 09 '18 at 15:28
What happens if a vector has length 0 and you copy it? and Operator = needs to `delete [] items` before you re assign. — Code Gorilla, Jan 09 '18 at 15:30
The `inline` keyword does not do what you expect in modern c++. Notably, compilers can ignore it (and most do) when deciding to inline a function. See https://stackoverflow.com/questions/1759300/when-should-i-write-the-keyword-inline-for-a-function-method — François Andrieux, Jan 09 '18 at 15:30
matrix.getRowCount() will crash if there are no rows, because of null pointer use. — Code Gorilla, Jan 09 '18 at 15:31
`vektor::operator[]` can throw `out_of_bounds`, why don't you have `matrix::operator[]` also throw in similar conditions? — Caleth, Jan 09 '18 at 15:47
@FrançoisAndrieux o, I didn't know this was possible, ill try that one and: are the functions broken then or just not inline — Raphael, Jan 09 '18 at 15:56
@Code Gorilla I was just testing the core functionality right now, these cases will be implemented soon(tm) — Raphael, Jan 09 '18 at 15:59
@Bathsheba I forgot to delete the existing vektor, thanks for pointing that out — Raphael, Jan 09 '18 at 16:00
@Raphael The `inline` keywords there are just not doing anything. It doesn't break anything. — François Andrieux, Jan 09 '18 at 16:02
@Raphael -- *I forgot to delete the existing vektor* -- You really don't need to do this in the assignment operator. You already have a copy constructor and a destructor -- use the [copy / swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) — PaulMcKenzie, Jan 09 '18 at 16:59

score 0 · Accepted Answer · answered Jan 09 '18 at 15:34

0

If selectedTab is 3 and you do tmp = menItems[selectedTab][i]; your code will seg fault because you are accessing outside the bounds of the memory.

I might have taken what you have written too literally and you change selectedTab

answered Jan 09 '18 at 15:34

Code Gorilla

667
8
20

1

Yeah, I feel a bit stupid that I did not see that. Thank you! – Raphael Jan 09 '18 at 16:02

Problems with self created template matrix class

1 Answers1