-1

I'm still very new to C++, and I'm having issues with allocating heap memory. This is what I have in my header file:

const int NUM_WORDS = 1253;
const int CHAR_SIZE = 256;

class CWords 
{
public:
    // constructor(s)
    CWords();

    // destructor
    ~CWords();

    // public member functions
    void            ReadFile();
    const char*     GetRandomWord() const;


private:
    char*  m_words[NUM_WORDS]; // NUM_WORDS pointers to a char
    int    m_numWords;         // Total words actually read from the file
};

I'm trying to allocate space in the implementation cpp file, but I can't (default constructor):

CWords::CWords()
{
        m_numWords = 0;
        m_words = new char[strlen(m_words) + 1];  // LINE 31
        strcpy(m_words, "NULL");    // LINE 32
}

line 31 gives me:

cannot convert 'char**' to 'const char*' for argument '1' to 'size_t strlen(const char*)'

and line 32 gives me:

cannot convert 'char**' to 'char*' for argument '1' to 'char* strcpy(char*, const char*)'

I don't know what these errors mean.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
NewB123
  • 1
  • 2
  • 3
    `new char[strlen(m_words) + 1]` attempts to create one single array of characters, not an array of strings. What you *really* want is `std::vector`. – Some programmer dude May 16 '22 at 07:12
  • `m_words = new char[strlen(m_words) + 1];` is wrong because `m_words` isn't a pointer type. It's already an array (of pointers). Nor does `strcpy(m_words, "NULL"); ` make any sense either. In fact, the comment `// NUM_WORDS pointers to a char` seems to announce you're already aware that `m_words` is an array of pointers, so *none* of the usage in that constructor is correct as-shown. Were you the one that wrote that comment?? – WhozCraig May 16 '22 at 07:13
  • 2
    If your assignment is to learn about arrays and pointers, then you seem to have skipped some rather important parts of your text-book or a few lessons. You might want to refresh the chapters or arrays and pointers in your text-books, or invest in [some good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282). – Some programmer dude May 16 '22 at 07:15
  • 2
    Dynamic allocation is not your issue here. First you need to understand what you are trying to do with pointers and arrays, before you can even get that far. – Lima Beans May 16 '22 at 07:16
  • everything on the header file is given and cannot change. I was trying to allocate space in head for 1253 times, for m_words[0] through m_words[1252] and set as NULL for each. – NewB123 May 16 '22 at 08:06

2 Answers2

2

The answer assumes that there is no strict requirement to force using C style arrays and strings.

As mentioned in the comments, even if you did manage the get rid of the bugs, this is not the recomended way to go in C++.

std::vector is the go-to container if you need a dynamic size array. std::array is for a static size one. Also it is better to use std::string than C style strings. Doing so will save you the trouble of manual memory management (and the bugs that come with it).

Below is a modification of the header of your class:

#include <vector>
#include <string>

class CWords
{
public:
    // constructor(s)
    CWords();

    // destructor
    ~CWords();

    // public member functions
    bool                ReadFile();
    std::string const & GetRandomWord() const;

private:
    std::vector<std::string>  m_words;
};

I'll leave the implementation for you.

Notes:

  1. ReadFile returns a bool in my version (not void). Reading from a file may fail, and so it is better to return false to indicate an error.
  2. If you ever consider to inherit from class CWord, you'd better make the destructor virtual. See here: Should every class have a virtual destructor?.
wohlstad
  • 12,661
  • 10
  • 26
  • 39
  • I cannot change the header file since the assignment wants me to use as is. Haven't gone over the vector yet in lecture yet so cannot use on this assignment. – NewB123 May 16 '22 at 08:03
0

if m_words should represent multiple strings

In this case m_words is a static array with pointers pointing to several strings.
Hence it makes sense to initialize those pointers with NULL in your constructor (e.g. using a for loop for NUM_WORDS entries).
Beside that initialize the member m_numWords with 0. Obviously this member should contain the info how many entries of m_words are already filled.

if m_words should represent 1 string

Use char m_words[NUM_WORDS]; in your header file
then you don't need to dynamically allocate memory in the constructor (the memory is then part of every instance's size).

Or simply use a char* m_words; in your header file
and dynamically allocate as much chars as you need at a later time. This way you can free your string and allocate it with a different size whenever needed.
However in this case it is strongly recommended to initialize m_words with a NULL pointer in constructor.

Please also have a look at the strlen() description. It can only count the characters, that are already contained in your string.
Use sizeof() to measure the bytesize of static arrays.

MattTT
  • 339
  • 3
  • 9
  • I cannot change the header file since the assignment wants me to use as is. What I was trying to do was to set NULL for each. I am trying to allocate space for 1253 in head, like [0], [1], ... [1252]. since I need to run the ReadFile function to read a file that has 1253 names. – NewB123 May 16 '22 at 08:01
  • In this case `m_words` obviously only contains pointers. You do not need to explicitly allocate the space for them (it is a static array). I updated my answer. Please have a look. – MattTT May 16 '22 at 09:30