0

After referencing the following resources: here and here. So I can see how the right way to do this is. Then after reading this post, I can see my previous warning was fixed through a typechar of char* "mystring" being passed into the argument of a function.

However, I am still getting the an error for a pretty intuitive couple lines of code (though I haven't touched c++ in some type, hence why I am having some trouble).

TextDocument.h

#ifndef ____TextDocument__
#define ____TextDocument__

#include <stdio.h>

class TextDocument {
    char *text;
    char *docName;
public:
    void SetText(char *otherText);
    char *GetText();
    void SetDocName(char *newName);
    char *GetDocName();
    int GetTextLength();
};

#endif /* defined(____TextDocument__) */

TextDocument.cpp

#include <iostream>
#include "TextDocument.h"
#include "string.h"
using namespace std;

void TextDocument::SetText(char *otherText){

    cout << otherText << endl;

    if (text != 0)
        delete text;          //free the memory

    text = new char[strlen(otherText)+1];   // +1 for the null char
    strcpy(text, otherText);                //text <- otherText
}

char *TextDocument::GetText(){
    return text;
}

void TextDocument::SetDocName(char *name){

    if (docName != 0)
        delete docName;

    docName = new char[strlen(name) + 1];   // +1 for the  \0 terminator
    strcpy(docName, name);                  // docName <- name
}

char *TextDocument::GetDocName(){

    return docName;
}

int TextDocument::GetTextLength(){
    if (text != 0) {
        return strlen(text);
    }
    else return 0;
}

main.cpp

#include <iostream>
#include "string.h"
#include "TextDocument.h"
#include "Folder.h"

using namespace std;

int main(void){

    TextDocument *sampleDoc;
    sampleDoc = new TextDocument;


    sampleDoc->SetText((char *)"some str"); // I have no idea why there is a linker error here.

    return 0;
}

run.sh 

g++ *.cpp -o main
./main

output:

Blakes-MacBook-Pro:data_encapsulation bmc$ sh run.sh 
some str
main(848,0x7fff7f54b300) malloc: *** error for object 0x8000000000000000: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
run.sh: line 2:   848 Abort trap: 6           ./main

Problem 1

Why isn't it deleting my char* text when it is uncreated.

Problem 2 (Sidebar problem, not my immediate issue)

Where is the best place to put all those .h files? Example) I need #include <iostream> and using namespace std inside of a few different .h or .cpp files, where would be the best place to put them; if you put them in only the main those other modules won't be able to see it and yield errors.

1st Fix Submission

So after screwing with this thing some more, I got the error to go away with by changing the line from

if (text != 0)
    delete text;          //free the memory

to

if (text)
    delete text;          //free the memory

I guess I understand the logic if (thestringeisntempty) delete text; but why doesn't if(text != 0) delete text; work as well?

Community
  • 1
  • 1
bmc
  • 817
  • 1
  • 12
  • 23
  • 1
    your problem is not using std::string but using the highly unrecommended new and delete. – The Techel Apr 30 '17 at 17:10
  • Do you set text to `NULL` (or `nullptr`) in the constructor of TextDocument? EDIT: nevermind, there is no constructor. – Phil M Apr 30 '17 at 17:11
  • @The Techel, so you just use a `std::string` data member and then just reset its value in there? I am trying to learn best practices. – bmc Apr 30 '17 at 17:12
  • intialize `text` to `NULL` – Sniper Apr 30 '17 at 17:14
  • This code hurts. Use `std::string` already! – Christian Hackl Apr 30 '17 at 17:57
  • I am having trouble conceptually understanding what is happening when I just say `char* text` without setting it to `NULL`. Shouldn't it already be `NULL`? Why do I have to explicitly do that in a constructor or `init()` function? – bmc Apr 30 '17 at 18:15
  • No, it won't already be NULL. C/C++ doesn't do that automatically. – Phil M Apr 30 '17 at 20:08

1 Answers1

1

Two solutions:

  1. Add a constructor to TextDocument that properly initializes your pointers.

    TextDocument() : text(nullptr), docName(nullptr) {}

Use NULL instead of nullptr if your compiler doesn't support the latter.

  1. Do away with char*s and use std::string.
Phil M
  • 1,619
  • 1
  • 8
  • 10
  • This works, but I am still confused why I have to initialize `char* text` or `std::string text` to something initially. Is it just like a block of uninitialized memory, void of even the NULL state? – bmc Apr 30 '17 at 18:16
  • you have to initialize the char* to something known so you can safely assert it's later on. Here it does mean that the pointer is currently not pointing to a valid char. Don't pass nullptr (=invalid pointer value) to the constructor of std::string and use the default constructor, it will leave the string object in an 'empty'-state. – The Techel Apr 30 '17 at 18:51