0

I'm refreshing how to write constructors in C++, but the C++ documentation version is throwing silly compiler errors.

I've looked at other examples from stack overflow and mimicked them and still similar errors

assume #includes are already here .h file

class Matrix{
    private:
    //eventually going to create 2d vector here
    public:
      // from: http://www.cplusplus.com/doc/tutorial/classes/
      Matrix (string);
      //another way shown from other examples
      Matrix (string filename); 
  };

.cpp

Matrix::Matrix(string filename){
    int num = 0;
    string line;
    ifstream matrix_file(filename);
    if(matrix_file.is_open()){
      while(getline(matrix_file, line)){
        stringstream extract(line);
        while(extract >> num){
          cout << num << " ";
        }
        cout << '\n';
      }
      matrix_file.close();
    }

  }

main.cpp

int main(int argc, char *argv[]){

    string filename = argv[1];
    Matrix grid (filename);
    return 0;
  }

I'm expecting upon object creation when the constructor is called, it prints out the values in the file. But when compiling, I get:

matrix.h:6:12: warning: unnecessary parentheses in declaration of ‘string’ [-Wparentheses]
     Matrix (string);
            ^
matrix.h:6:19: error: field ‘string’ has incomplete type ‘Matrix’
     Matrix (string);
                   ^
matrix.h:2:7: note: definition of ‘class Matrix’ is not complete until the closing brace
 class Matrix{

or

matrix.h:6:19: error: expected ‘)’ before ‘filename’
     Matrix (string filename);
            ~      ^~~~~~~~~
                   )
main.cpp: In function ‘int main(int, char**)’:
main.cpp:11:24: error: no matching function for call to ‘Matrix::Matrix(std::__cxx11::string&)’
   Matrix grid (filename);

depending on which way I initialize the string parameter in .h file. I'm thinking I have a small typo somewhere, but I'm not finding anything wrong with this simple piece of code. Any help would be much appreciated. Thanks

Improve this question
3
  • 3
    assume #includes are already here .h file ... seems to be an incorrect assumption. Do you have using std ? Commented Oct 2, 2019 at 1:59
  • 1
    you need to include<string> and scope it with std since string is also part of the std library. You can do ‘using namespace std’ for convenience. It also looks like you have invalid constructor prototypes/definitions in header file. Just need to reference the parameter types, not the actual argument as well. So just Matrix(string) Commented Oct 2, 2019 at 2:04
  • omg, that was it. I completely forgot about that. Didn't include #include because I didn't think they were relevant. I was so wrong. I was looking at the wrong piece of the code, lol. Thank you very much. I did have using namespace std in the .cpp for using iostream, just needed to include the string library, but included in .h since matrix.cpp and main.cpp both need to use it. Thank you so much guys Commented Oct 2, 2019 at 2:09

2 Answers 2

Reset to default
2

There are a few problems with your code.

Firstly, you haven't included <string>. You will also need to include <sstream>, <fstream> and <iostream>. It's not considered good practice to include a whole namespace with using namespace std. So prefix types from the standard library with std:: - i.e. std::string, std::ifstream, std::stringstream and std::cout.

Secondly, you have duplicate constructors taking one std::string parameter:

Matrix(string);

Matrix(string filename);

You will get a compiler error for the second line saying the member function is already defined or declared. That's because you're trying to define the same constructor twice. It is optional to use parameter names for functions in the header file. However, for readability it is best to have them there. So I would just go with:

Matrix(std::string filename);

You could change this line to accept a const std::string& in order to avoid copying. However, that's up to you.

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

1 Comment

thank you very much for your thorough explanation. I had all those includes listed, except for <string>. That fixed it. I knew I had duplicate constructors there. That was shown for demonstration that either one I tried didn't work. Good to know about not including the whole namespace. The general rule I like to follow if if I use a ton of stds, I would simplify my code by just including it on top, otherwise, I'll add it as needed. I'll get in a habit of not including on top though. Thanks again
2

you need to include <string> and scope it with std:: since string is also part of the std namespace. You can do using namespace std for convenience. For constructor definition in header, i think it’s cleaner to omit the parameter name and just have the type.

Improve this answer

3 Comments

ah ok thank you. Other examples I saw did Matrix (string somevariable), which probably works, but I like the simplified Matrix (string), which is what cplusplus.com demonstrates.
I guess it depends on how you are defining your constructor. If you have your constructor in a implementation file, then I think there is not need to have the parameter in there like Matrix(string param) in the header file. Instead it can just be like this Matrix(string) then in implementation file, build out the full constructor with the params. If your constructor is in header file, then I guess you would need to add the params.
Unless it's "very easy" to understand even for a nobrainer, omiting names in a declaration makes the function cryptographic.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.