Initialization in class - C++

[HelpMeC]

This is some of the fundamental methods in my Matrix class.
I'm not sure what I have done with initEntries and/or initCopy is so elegant, and maybe there is some flexibility I should use and avoid these helper functions.

The partial CPP file:

#include "Matrix.h"


void Matrix::initEntries()
{
    for (int i = 0; i < m_Dims.rows * m_Dims.cols; i++)
    {
        m_Entries[i] = 0;
    }
}


void Matrix::initCopy(const Matrix &m) // I use this function also in operator= method!
{
    for (int i = 0; i < m_Dims.rows * m_Dims.cols; i++)
    {
        m_Entries[i] = m.m_Entries[i];
    }
}


Matrix::Matrix(int rows, int cols)
    : m_Dims{rows, cols}, m_Entries(new float[rows * cols])
{
    if (m_Dims.rows < 0 || m_Dims.cols)
    initEntries();
};



Matrix::Matrix(const Matrix &m) // Copy constructor
    : Matrix(m.m_Dims.rows, m.m_Dims.cols)
{
    initCopy(m);
}

This is the header file:

#ifndef MATRIX_H
#define MATRIX_H
#include <iostream>


const std::string errMsg = "ERROR: ";


const std::string doubleSpace = "  ";


const std::string oneSpace = " ";


const std::string doubleAster = "**";


const float lowProb = 0.1f;


/**
 * @struct MatrixDims
 * @brief Matrix dimensions container
 */
typedef struct MatrixDims
{
    int rows, cols;
} MatrixDims;


class Matrix
{
private:
    MatrixDims m_Dims;
    float *m_Entries;
    void initEntries();
    void initCopy(const Matrix &m);
public:
    Matrix(int rows = 1, int cols = 1);
    Matrix(const Matrix &m);
    ~Matrix();
    int getRows() const;
    int getCols() const;
    Matrix &vectorize();
    void plainPrint() const;
    Matrix &operator=(const Matrix &other);
    Matrix operator*(const Matrix &m) const;
    Matrix operator*(const float &m) const;
    Matrix operator+(const Matrix &m) const;
    Matrix &operator+=(const Matrix &m);
    float &operator()(int i, int j);
    float operator()(int i, int j) const;
    float &operator[](int i);
    float operator[](int i) const;


    friend std::istream &operator>>(std::istream &s, const Matrix &m);
    friend std::ostream &operator<<(std::ostream &s, const Matrix &m);
    friend Matrix operator*(const float &c, const Matrix &matrix);
};


std::istream &operator>>(std::istream &s, const Matrix &m);
std::ostream &operator<<(std::ostream &s, const Matrix &m);
Matrix operator*(const float &c, const Matrix &matrix);


#endif //MATRIX_H

[laserlight]

The variables that you have declared in the header are problematic: those are definitions, which means that if you include the header in more than one source file, you'll break the one definition rule. What you can do is to use the inline keyword, e.g.,

inline const std::string errMsg = "ERROR: ";

But you may have to compile with respect to C++17. On the other hand, these string constants look like implementation detail, so perhaps they should go in a source file instead. Remember to #include <string> when you want to use std::string.

Your Matrix constructor should be declared explicit because it can be called with just one argument, and you probably don't want that to look like:

Matrix m = 5;

Furthermore, you might want to check the if statement logic: if it is supposed to do a sanity check, maybe it should throw an exception on failure.

Your overloaded operator>> is wrong: the second parameter needs to be a non-const reference.

Furthermore, you don't actually need to #include <iostream> just to declare the overloaded operator >> and <<. Rather, #include <iosfwd>, then when you want to implement them in a source file, #include <istream> and <ostream>. <iostream> contains declarations for std::cout, std::cin etc, which you don't need when it's purely for overloading for istream and ostream.

As for your helper functions: they seem okay to me. You could use generic algorithms to simplify, but that isn't terribly important.

[HelpMeC]

@laserlight -
As for the inline issue - I don't know what the intent of inline here is, as we didn't learn about that more than 4 minutes.
What's actually the problem with defining these consts as is, without inlines?...

Secondly, there are some classes which use header of Matrix and its consts as well.

Very beneficial remarks.
Thank you!!!


BTW, I've added the explicit keyword before the copy constructor:

explicit Matrix(const Matrix &m);

But the compiler yells - Copy constructor should not be defined as explicit...

[laserlight]

What's actually the problem with defining these consts as is, without inlines?

The same problem with defining non-inline functions in header files. Try including the header file in two source files, compile and then try to link them together to form one program. You should find that you end up with a link error.

The problem is that these global constants end up defined more than once in the program, breaking the rule that they can be defined only once in the entire program. Traditionally, the solution is to declare them extern in the header (so the declaration ceases to be a definition) and move their definitions to a single source file, but that was a pain point for library authors trying to write header-only libraries. Allowing the inline keyword for defining objects eliminated that pain point such that you can declare the variable to be inline in the header, and since that definition would be consistent across the program (as long as you play by the rules and include the header where needed, just like for inline functions), it's fine. Unfortunately, this is a fairly new addition to the language, so you may find that your compiler doesn't support it by default, or might not even support it at all.

BTW, I've added the explicit keyword before the copy constructor:

But the compiler yells - Copy constructor should not be defined as explicit...

Declaring a constructor as explicit means that you want it to be explicitly invoked for type conversions. This is great to prevent the example I showed you, or other cases like writing a function that accepts a Matrix argument, only to find that passing an int as the argument works too (i.e., the constructor is implicitly invoked to convert the int to a Matrix)!

But a copy constructor doesn't perform a type conversion; it performs a copy. Therefore, you pretty much have no reason to declare a copy constructor as explicit.

EDIT:
Also, I noticed that you declared the non-member operator* as a friend. Generally, you should prefer to write non-member non-friend functions. Of course, you do need some member functions, and sometimes you do need some friend functions, but if a function can be efficiently implemented as a non-member non-friend function, that's what you should do. This cuts down on the number of functions with access to the internals of the class, making it easier to change the internals of the class later.

So, to implement that non-member operator* that multiplies by a float what I would do is define a member operator *= that changes the current matrix. Then, I would define two non-member operator*: one with the matrix on the left hand side and the other with the matrix on the right hand side. Both of them would copy the matrix and change the copy using the member operator*= and return the result. (This is efficient because copy elision could be done by the compiler such that the copy only happens once, and even when it cannot be done, move semantics come into play to still make returning a copy of the modified local copy efficient... but you'll have to learn about that first.)

You presumably would not want to do this "implement operator* using operator*=" appproach for the version of operator* that takes two matrices since the result is a third matrix potentially of different dimensions than the two, so it's perfectly fine for that operator* to be a member.

On the other hand, you can adopt the same approach for adding two matrices, i.e., implement a member operator+= to add another matrix to the current matrix, then implement a non-member operator+ to add two matrices to produce a third matrix.

[HelpMeC]

@laserlight -
As for the explicit issue - so where did you mean I have to declare the constructor as explicit?
Why the copy constructor doesn't have to be defined as explicit just because it's a copy constructor? Your example, to my understanding comes to demonstrate a problem when the copy constructor is not defined explicit:
You have written, in this case, one can invoke the copy constructor this way:

Matrix m = 5; // Here the compiler will try implicitly convert 5 (which is int) to a Matrix copy - which will be done in run-time error.

I don't know really what move semantics is, but I think I understood the majority from your remark. I'll take it to my consideration.
Tomorrow the next project is released

[laserlight]

As for the explicit issue - so where did you mean I have to declare the constructor as explicit?

Right over here:

Matrix(int rows = 1, int cols = 1);

Why the copy constructor doesn't have to be defined as explicit just because it's a copy constructor?

I already explained why in my previous post.

You have written, in this case, one can invoke the copy constructor this way:

Matrix m = 5; // Here the compiler will try implicitly convert 5 (which is int) to a Matrix copy - which will be done in run-time error.

No, the copy constructor will do no such thing. It cannot do that: as you correctly observed, 5 is an int, not a Matrix. You cannot copy construct a Matrix from an int. It is impossible by definition. It can never ever be done, no matter how hard you try.

What you can do is construct a Matrix from an int using some other constructor, and then copy construct another Matrix from this first Matrix. So what you're looking at is the invocation of this constructor:

Matrix(int rows = 1, int cols = 1);

What happens is that cols is provided with the default argument of 1, then rows is set to 5, so this:

Matrix m = 5;

is equivalent to:

Matrix m{5, 1};

except that the former looks confusing because it looks like you're setting an entire Matrix to be the scalar 5. Using the explicit keyword prevents this.

Another approach, which may be even better in this case, is to ditch the default arguments and instead define a default constructor that delegates to the two-parameter constructor to construct a 1 by 1 Matrix.

[HelpMeC]

@laserlight -
Wow How did you learn this lang?! For every rule here there are 100 exceptions!

I have learnt the implicit conversion which exists in C++ - which says every time we try assigning ((operator=)/copy(copy constructor) which are somehow equivalent) some value to another - if the value from right is not exactly the same as the one from right - the compiler will try to implicitly convert (cast) the right one to the type of the left one.

Now, you show me something super weird - this assignment of integer to matrix, actually invokes not the copy constructor but the default constructor, for some mysterious reason, and it is actually able to be compiled when passing only one of the arguments (which seems reasonable of course).

[laserlight]

This might be a more striking example:

#include <iostream>

struct X
{
    X(int a=0, int b=1) : a(a), b(b) {}
    int a, b;
};

void foo(const X& x)
{
    std::cout << "foo: " << x.a << ", " << x.b << std::endl;
}

int main()
{
    foo(2);
}

It looks like foo(2) should result in a compile error since foo has a const reference to X parameter, but it compiles perfectly fine because the constructor for X allows for the implicit conversion from int to X.

Now, sometimes this is actually what you want, e.g., if you're writing an arbitrary precision integer class, it might be convenient to implicitly convert 2 to your class type. But not here. So, the solution is to require an explicit conversion by declaring the constructor as explicit. foo(2) would then result in a compile error, but this would work:

foo(X{2});

[HelpMeC]

@laserlight -
Wow How did you learn this lang?! For every rule here there are 100 exceptions!

I have learnt the implicit conversion which exists in C++ - which says every time we try assigning ((operator=)/copy(copy constructor) which are somehow equivalent) some value to another - if the value from right is not exactly the same as the one from right - the compiler will try to implicitly convert (cast) the right one to the type of the left one.

Now, you show me something super weird - this assignment of integer to matrix, actually invokes not the copy constructor but the default constructor, for some mysterious reason, and it is actually able to be compiled when passing only one of the arguments (which seems reasonable of course).

Maybe you some remarks on this comment of mine, a minute before your new comment?
Now I'll go through your new example

thank you!!!!!!!!!!!!!!!!!!!!!!!!!!

[HelpMeC]

@laserlight -
Got your example. I'm glad you also show me such examples as our final exams like to ask many questions about weird cases like that. Thank you so much!

Last question:
How do I know when the compiler invokes the copy constructor or the default constructor?
I have heard many times that when we pass arguments like that:

Matrix s(2, 2);
Matrix m(s); // The copy constructor is invoked, sometimes the default constructor is invoked
Matrix m = s; // The copy constructor is invoked, sometimes the default constructor is invoked

How can I distinguish which invocation is about to occur?

Thank you again.

[laserlight]

The default constructor is not invoked in any of those three examples. Remember, the default constructor is the one that is invoked without any arguments, whereas all three involve arguments.

To distinguish which constructor will be invoked, you have to example the number and type of the arguments and match that with the parameter lists of the various constructors.

So, the first one would best match the constructor that has two int parameters. It so happens that there is just such a constructor, and while that same constructor could be invoked as the default constructor, in this context it doesn't act as the default constructor.

For the next two, the argument is an existing Matrix object, so the copy constructor is invoked.

[HelpMeC]

@laserlight -
So in your previous example:

#include <iostream>

struct X
{
    X(int a=0, int b=1) : a(a), b(b) {}
    int a, b;
};

void foo(const X& x)
{
    std::cout << "foo: " << x.a << ", " << x.b << std::endl;
}

int main()
{
    foo(2);
}

The default constructor is invoked (Generally, as far as the parameter is not of the exact same type - the compiler is looking for a suitable constructor) and not the default copy constructor, because 2 is not X?
And if foo's parameter weren't a reference, and we passed to foo an existing X's instance - the copy constructor would be invoked?

Thank you.

[laserlight]

Yes and yes

[HelpMeC]

Really appreciate your help, you're amazing.
Thanks.