0

When I am trying to assign a member variable, which is const char*, it just ends the program. However, if I get some cin inputs before I assign a member variable, it works. this is my NameCard.h

class NameCard {

private:
    const char* name;
    const char* companyName;
    const char* phoneNumber;
    enum grade{CLERK, SENIOR, ASSIST, MANAGER};
    grade my_grade; 

    NameCard* nextCard; 
    NameCard* head;

public:
    NameCard(const char*, const char*, const char*, int);
    void setName();
    void setCompanyName();
    void setPhoneNumber();
    void setMyGrade();
    void setHead(NameCard*);
    void setNextCard(NameCard*);

    void findInfo(char*);
    void printAll();

};

This is main.cpp

#include <iostream>
#include <cstring>
#include "NameCard.h"

using namespace std;
NameCard::NameCard(const char* a, const char* b, const char* c, int d){
    name = a;
    companyName = b;
    phoneNumber = c;
    switch(d){
        case CLERK: my_grade = CLERK; break;
        case SENIOR: my_grade = SENIOR; break;
        case ASSIST: my_grade = ASSIST; break;
        case MANAGER: my_grade = MANAGER; break;
    }
    cout << name << " " << companyName << " " << phoneNumber << " " << my_grade << endl;
}
void NameCard::setName(){
    char* name;
    cout << "type name." << endl;
    cin >> name;
    this->name = name;
    cout << this->name << endl;
}

void NameCard::setCompanyName(){
    char* company_name;
    cout << "type company name." << endl;
    cin >> company_name;
    this->companyName = company_name;
    cout << this->companyName << endl;
}
void NameCard::setPhoneNumber(){
    char* phone_number;
    cout << "type phone number." << endl;
    cin >> phone_number;
    phoneNumber = phone_number;
    cout << phoneNumber;
};
void NameCard::setMyGrade(){
    int input_grade;
    cout << "type position. 1)CLERK 2) SENOIR 3) ASSIST 4)MANAGER" << endl;
    cin >> input_grade;
    switch(input_grade){
        case CLERK: my_grade = CLERK; break;
        case SENIOR: my_grade = SENIOR; break;
        case ASSIST: my_grade = ASSIST; break;
        case MANAGER: my_grade = MANAGER; break;
    }
    cout << input_grade;
};

void NameCard::setHead(NameCard* head){
    this->head = head;
};

void NameCard::setNextCard(NameCard* next){
    this->nextCard = next;
}

void NameCard::findInfo(char* target_name){
    NameCard* temp;
    temp = head;
    while(temp != NULL){
        if(temp->name == target_name){
            cout << "name : " << temp->name << endl;
            cout << "companyName : " << temp->companyName << endl;
            cout << "phoneNumber : " << temp->phoneNumber << endl;
            cout << "grade : " << temp->my_grade << endl;
            break;
        }
        temp = temp->nextCard;
    }
};

void NameCard::printAll(){
    NameCard* temp;
    temp = head;
    while(temp!= NULL){
        cout<< "----------------------------" << endl;
        cout << "name : " << temp->name << endl;
        cout << "companyName : " << temp->companyName << endl;
        cout << "phoneNumber : " << temp->phoneNumber << endl;
        cout << "grade : " << temp->my_grade << endl;
        temp = temp->nextCard;
    }
}


int main(){
    int index = 0;
    NameCard* head = new NameCard("test", "test", "test", 3);
    NameCard* bef = NULL; 
    // char input[50];
    // cout << "nmae ";
    // cin >>input; 
    head->setName();
    head->setPhoneNumber();
    head->setCompanyName();
    head->setMyGrade();
    
    return 0;
}

And the problem is here. This is not working. The program ends right after it gets name from setName(). It just ends at the setName() cin >> name. it did not print name after then, and stopped.

int main(){
    int index = 0;
    NameCard* head = new NameCard("test", "test", "test", 3);
    NameCard* bef = NULL; 
    // char input[50];
    // cout << "nmae ";
    // cin >>input; 
    head->setName();
    head->setPhoneNumber();
    head->setCompanyName();
    head->setMyGrade();
    
    return 0;
}

However, this is working fine. it gets the test input and executes all the code below.

int main(){
    int index = 0;
    NameCard* head = new NameCard("test", "test", "test", 3);
    NameCard* bef = NULL; 
    char input[50];
    cout << "test input ";
    cin >>input; 
    head->setName();
    head->setPhoneNumber();
    head->setCompanyName();
    head->setMyGrade();
    
    return 0;
}
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
yongseunglee
  • 73
  • 2
  • 8
  • 3
    Is there a particular reason for using pointers? They shouldn't be necessary with `std::string` and a standard container, and they're causing all sorts of trouble. – chris Sep 11 '20 at 02:39
  • It is a pointer, you cannot modify `char *` by assignment, maybe you can try `strcpy` – Zeus Sep 11 '20 at 02:40
  • @chris umm... just obsession.. I do not know why the code above is not working... anyway, what is the troubles you mean? – yongseunglee Sep 11 '20 at 02:42
  • @ZhuSong Really? but the assignment is working fine in the setName() function when I get the cin before execute setName() function. – yongseunglee Sep 11 '20 at 02:43
  • When you execute `cin >> name;` in `NameCard::setName`, the local variable has not been initialized. Anything can happen. – 1201ProgramAlarm Sep 11 '20 at 02:44
  • 3
    Trash that obsession. `char*` is *nothing but trouble* and will only bring suffering and misery to your code. Use `std::string` to avoid a whole world of hurt relating to memory management, buffer overflows, and having to allocate and re-allocate memory all the time. – tadman Sep 11 '20 at 02:47
  • 1
    @yongseunglee, Reading into an uninitialized pointer and not cleaning up memory are two that jump out at me immediately, not to mention copy/move operations not being overridden. In addition, the code is harder to follow for no discernable benefit. If you're learning to use owning pointers, I suggest limiting it to things that owning pointers are used for—create a string or container or smart pointer class that you can reuse elsewhere if you desire. That way you avoid mixing ownership code with business logic. – chris Sep 11 '20 at 02:47
  • @1201ProgramAlarm should I initialize name? Then which value should I assign to the name? just trash value? – yongseunglee Sep 11 '20 at 02:47
  • It is time to stop experimenting and do some book-learning on [pointers](https://en.cppreference.com/w/cpp/language/pointer), [objects](https://en.cppreference.com/w/cpp/language/object) (no, they probably aren't what you think they are), and [lifetimes](https://en.cppreference.com/w/cpp/language/lifetime). This is not overly complicated stuff, but it is very finicky and utterly unforgiving. Very simple mistakes have [very chaotic results](https://en.cppreference.com/w/cpp/language/ub), making pointer use very hard to learn through trial and error. – user4581301 Sep 11 '20 at 02:51
  • *However, if I get some cin inputs before I assign a member variable, it works.* -- "When I drove a car without a driver's license, I didn't get stopped by a policeman." – PaulMcKenzie Sep 11 '20 at 02:52
  • 1
    Chris is pointing in the direction of [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii), an absolutely vital concept to understand when writing programs in C++. – user4581301 Sep 11 '20 at 02:56
  • 1
    Thanks @user4581301 I should do some book-learning... – yongseunglee Sep 11 '20 at 02:58
  • @yongseunglee -- `setName()`, `setCompanyName`, `setPhoneNumber` -- all three functions attempt to write to uninitialized pointers. Writing to an uninitialized pointer is *undefined behavior*. Thus the program may work for years, may crash right away, may work today and not tomorrow, may work on your computer and not your friend's computer, may work on a thousand computers and crash on computer 1001, etc. To prevent having programs that behave like this, you don't put yourself in a position of writing programs like this. That's why `std::string` (in this case) should be used. – PaulMcKenzie Sep 11 '20 at 03:02

1 Answers1

2

You are writing to memory you didn't allocate and most likely don't own. Check this for example

void NameCard::setName(){
    char* name;
    cout << "type name." << endl;
    cin >> name;
    this->name = name;
    cout << this->name << endl;
}

You declare char* name and try to read from cin into it. But name is just an uninitialized pointer.

One fix may be to declare name as an array, which also decays to a pointer (so it can be used as a pointer), but does have associated memory. Like this

char name[50];

Or if you want it to be dynamically allocated

char* name = new char[50];
// Use it and when you are done delete it
// Never forget to release memory allocated with new or you get a memory leak
delete[] name; 

The key point is that the pointer needs to point to some memory block you know you can use. Note that in the example I allocated 50 chars, you have to make sure you never use (read/write) past the allocated memory, or you get the same error you have been getting and potentially program termination.

A safer alternative is to use std::string, which automatically handles memory for you, so you don't have to mind about allocation and releasing. You could do something like

std::string name;
std::cin >> name;
DeltA
  • 564
  • 4
  • 12
  • 1
    And of course when you bring up `delete[]`, you now have to discuss what happens if you use it on the `const char *` assigned with `NameCard("test", "test", "test", 3);` The problems with raw dynamic allocations just go on and on and on... – user4581301 Sep 11 '20 at 03:06
  • @user4581301 I just intend to give a head-start. Feel free to improve on the answer if you like. – DeltA Sep 11 '20 at 03:10
  • Understood. That comment is intended to nudge future readers in the direction of `std::string`. You just can't win with unmanaged dynamic allocation. It's a rabbit hole. – user4581301 Sep 11 '20 at 03:13
  • Try to use std::getline() instead – kesarling He-Him Sep 11 '20 at 03:14