-3

Hello I am just learning C++. I have this code but when I try to build the solution error from title shows up. Any ideas how to fix that?

class cMan
{
public:
    cMan(char *chFirstName, char *chLastName, double dWeight, int iHeight);
    ~cMan();
    void vWriteMembersValues();

private:
    char chFirstName[5];
    char chLastName[5];
    double dWeight;
    int iHeight;
    
};

cMan::cMan(char *chFirstName, char *chLastName, double dWeight, int iHeight)
{
    for (int i = 0; i < 5; i++)
    {
        this->chFirstName[i] = chFirstName[i];
        this->chLastName[i] = chLastName[i];
    }
    
    this->dWeight = dWeight;
    this->iHeight = iHeight;
}

cMan::~cMan()
{
    delete this;
}

void cMan::vWriteMembersValues()
{
    for (int i = 0; i < 5; i++)
    {
        cout << this->chFirstName[i];
        
    }
    
    for (int i = 0; i < 5; i++)
    {
        cout << this->chLastName[i];
        
    }
    
    cout << " " << this->dWeight;
    cout << " " << this->iHeight;
    
}
int main()
{
    cMan cmI("Michal", "Stanko", 83.5, 200);
    
    cmI.vWriteMembersValues();
    
    cout << endl;
    
    cmI.~cMan();
    
    cin.get();
    cin.get();
    
    return 0;
}

I don't know much about programming but if anyone would explain this to me as easy as possible i would be glad.

General Grievance
  • 4,555
  • 31
  • 31
  • 45
Michal Stanko
  • 1
  • 2
  • 3
    Did your C++ book say to use `cmI.~cMan();`? If so, get a better book. – Thomas Matthews Mar 04 '22 at 21:14
  • What book are you using? Does it say to always use `this->` to access members? – Thomas Matthews Mar 04 '22 at 21:16
  • Wait a sec... `this` **isn't** the way? The Mandalorian lied? – user4581301 Mar 04 '22 at 21:17
  • See `strcpy` or better, use `std::string` for your text. – Thomas Matthews Mar 04 '22 at 21:18
  • You have a `delete` statement. Where's the associated `new`? – Thomas Matthews Mar 04 '22 at 21:21
  • 1
    More interesting is `delete this;` Effectively this says "delete the current object." Inside a destructor you have an infinite loop. The object calls to have the object destroyed inside a function called as a result of destroying the object. There is no way to do this correctly. Fortunately you don't have to. `this` is already being destroyed. Remove the line. Probably remove the destructor entirely because the [Rule of Zero](https://en.cppreference.com/w/cpp/language/rule_of_three) suggests that if the destructor does nothing, let the compiler define it. – user4581301 Mar 04 '22 at 21:33

3 Answers3

4

String literals are const char[] arrays, so you need to update your cMan() constructor to take const char* pointers instead of char* pointers. Or else, use std::string instead.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
2

The issue is const.

Try taking a const char * instead of a char *

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Ronen
  • 195
  • 7
  • 2
    General advice: always start with `const` data unless you know you will be modifying it. It helps prevent bugs and often allows the compiler to optimize more aggressively and produce a more efficient program. – user4581301 Mar 04 '22 at 21:15
  • @user4581301 Yes, you're right. Here's proof: No const: https://godbolt.org/z/h5rf3vanx With const: https://godbolt.org/z/c5hTaKPds Although is should be noted that the only thing that affects computational performance when using const is the manner in which the variable was originally declared. In the proof that I gave, if I change the function signature to const, that doesn't help performance: https://godbolt.org/z/qqvoPKG6r That's because the implementation of the function "change_my_int" could legally use const_cast – Ronen Mar 04 '22 at 21:41
2

You are calling the constructor

cMan cmI("Michal", "Stanko", 83.5, 200);

passing to it string literals.

In C++ opposite to C string literals have types of constant character arrays. Used as argument expressions they are implicitly converted to pointers to their first characters of the type const char *

Thus if you are going to use string literals as arguments of the constructor then declare it like

cMan( const char *chFirstName, const char *chLastName, double dWeight, int iHeight);

Pay attention to that the used string literals in this declaration

cMan cmI("Michal", "Stanko", 83.5, 200);

have more characters then the corresponding initialized data members of the class.

If you will pass string literals with length that less than the size of the arrays declared as data members then this loop

for (int i = 0; i < 5; i++)
{
    this->chFirstName[i] = chFirstName[i];
    this->chLastName[i] = chLastName[i];
}

can be a reason of undefined behavior when you will try to use the data members.

So to avoid the undefined behavior it is better to store strings in the data members. You could declare the data members as having the type std::string instead of the type char[5].

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335