kmiklas
kmiklas

Reputation: 13453

How to initialize a private char * in a Class constructor with user input upon instantiation

The following program crashes in the constructor definition. Apparently I can't use the constructor to copy the given string into the brand variable into the private class variable _brand.

Where am I going wrong here? The idea is to take user input for the brand name, and instantiate the class with this name as _brand.

EDIT: Please don't advise use of the std::string class, as I'm stuck with c-strings on this one. This is a tiny piece of an assignment for an intermediate C++ University class. The Professor wants us to have a foundation in C-strings before we move on to the STL string class. The instructions specifically forbid the use of std::string. Thx.

#include <iostream>
#include <cstring>

class Cheese {
    char * _brand;
public:
    Cheese(const char *); // constructor declaration
    char * getBrand();
};
Cheese::Cheese(char const * b) { // constructor definition
    strcpy(_brand, b); // CRASHES HERE
}
char * Cheese::getBrand() {
     return _brand;
}
int main(int argc, const char * argv[]) {
    const char * brand = "Cabot";
    Cheese * cheddar = new Cheese(brand);
    std::cout << cheddar -> getBrand();
    return 0;
}

Upvotes: 2

Views: 4618

Answers (4)

Jaka Konda
Jaka Konda

Reputation: 1436

As other have pointed out, _brand is just a pointer. It doesn't store a string, it points to a memory address where the strings starts. That memory space is something you must reserve.

The solution is quite simple:

_brand = new char[strlen(b)+1]; // Reserve memory chunk for your copied string. Add +1 for string terminator '\0'
strcpy(_brand, b); // "CRASHES HERE" should be gone now

By saying new, _brand variable now points to a valid chunk of memory (owned by your program), large enough to store your copied string.

At the end, don't forget to free _brand to avoid memory leaks.

Upvotes: 4

sampie Dev
sampie Dev

Reputation: 1

You cannot copy the data because in your class declaration you have just reserved a pointer, but you have not set it to point at anything. What I mean is when you call your constructor the object will just have a size of 4 bytes in memory which will hold a pointer to nowhere.

to solve this you can change the declaration of your brand variable into a char[] array but it a bad solution.

Upvotes: 0

Peter
Peter

Reputation: 5738

In C and C++ a pointer points to a location in memory. Accessing the pointer allows you to read to and write from the location it points to. In your program the member _brand is a pointer that hasn't been initialized, it's not defined where it points to. Reading from and writing to an uninitialized pointer is undefined behavior, which means everything and nothing can happen. This often means the program will crash, but because the behavior is undefined the outcome can be far worse - it could really write to a random location in memory, for example.

You need actual memory instead of just a pointer. Here are 3 reasonable ways:

Array

#include <iostream>
#include <cstring>

class Cheese {
    char _brand[256];
public:
    Cheese(const char *); // constructor declaration
    char * getBrand();
};
Cheese::Cheese(char const * b) { // constructor definition
    strncpy(_brand, b, sizeof(_brand)); // copies up to 255 characters, plus terminating \0
}
char * Cheese::getBrand() {
     return _brand;
}
int main(int argc, const char * argv[]) {
    const char * brand = "Cabot";
    Cheese * cheddar = new Cheese(brand);
    std::cout << cheddar -> getBrand();
    delete cheddar;
    return 0;
}

Pointer

#include <iostream>
#include <cstring>

class Cheese {
    char * _brand;
public:
    Cheese(const char *); // constructor declaration
    ~Cheese(); // destructor declaration
    char * getBrand();
};
Cheese::Cheese(char const * b) { // constructor definition        
    _brand = new char[strlen(b)+1];
    strcpy(_brand, b);  
}
Cheese::~Cheese() { // destructor definition 
    if (_brand != nullptr) // if you add another constructor that doesn't set _brand, make sure to set _brand to nullptr
        delete _brand; // if we don't free here there's a memory leak
}
char * Cheese::getBrand() {
     return _brand;
}
int main(int argc, const char * argv[]) {
    const char * brand = "Cabot";
    Cheese * cheddar = new Cheese(brand);
    std::cout << cheddar -> getBrand();
    delete cheddar; 
    return 0;
}

Smart Pointer

#include <iostream>
#include <cstring>
#include <memory>

class Cheese {
    std::unique_ptr<char> _brand; 
public:
    Cheese(const char *); // constructor declaration
    char * getBrand();
};
Cheese::Cheese(char const * b) { // constructor definition        
    _brand = new char[strlen(b)+1]; // smart pointer will free any memory assigned to it on destruction
    strcpy(_brand, b);  
}
char * Cheese::getBrand() {
     return _brand.get();
}
int main(int argc, const char * argv[]) {
    const char * brand = "Cabot";
    Cheese * cheddar = new Cheese(brand);
    std::cout << cheddar -> getBrand();
    delete cheddar;
    return 0;
}

Upvotes: 1

Geza Lore
Geza Lore

Reputation: 541

_brand is just a pointer and has no memory allocated behind it when your class is created. At the point when you try to do strcpy to it, it will likely be NULL or point to memory that your program has no access to and hence it crashes. You can use strdup which allocates the memory, but then you will have to free this later, for example in the destructor. Best is to use the standard C++ std::string.

Upvotes: 3

Related Questions