Struggling with C-string method again

[jetm0t0]

Ok so I am using a class MyString, and I am having a hard time properly writing a readLine function that is supposed to read from a file and display it without the specified delimiting character. example:
Class member void readline function (public): The readline() function will allow the client programmer to specify the delimiting character (the character at which reading will stop). It should work just like the getline() function works for C-strings; that is, it should place everything up to but not including the delimiting character into the calling object, and it should also consume (and discard) the delimiting character. This will be a void function that will take two arguments, a stream, and the delimiting character. It should not skip leading spaces. The limit of 127 characters imposed on the >> function above also applies to this function.

I also have a bit more overload functions to write, <, > <= , >=, [], but I think these should be easier. I'll post my .h, .cpp, and main.cpp Also I am not allowed to using anything from include <string>
My current error or issue, I was briefly told for my readline function when you add to a char array, you have to delete the old array and assign a new bigger array and then copy it to the new array, and then you can work with the new array as needed, so the error is my readline function with getline, I'm not sure how to write it better.

#pragma warning(disable:4996) // for Vis studio
#pragma once
#include <string>
#include <iostream>

using std::ostream; using std::istream;

static const int MAX_INPUT_SIZE = 127;

namespace cs_mystring
{
    class MyString
    {
    public:
        MyString(); // default constr
        MyString(const char *inString); // param constr
        MyString(const MyString &copyMe); // deeeep copy, check for self assignment?
        ~MyString(); // destruct
        friend std::ostream &operator << (std::ostream &out, const MyString &printMe); // print should be &? because it's an obj?
        // make sure to include friend inside cs_mystring namespace block
        friend std::istream &operator >> (std::istream &in, MyString &readMe);
        friend MyString operator + (const MyString leftOp, const MyString rightOp);
        MyString operator += (const MyString rightOp);

        char operator[](int index) const;
        char& operator[](int index);
        // use strcmp() for operator overloads and the big 3, desctruct,
        // overload = , copy constr
        friend bool operator < (const MyString leftOp, const MyString rightOp);
        friend bool operator <= (const MyString leftOp, const MyString rightOp);
        friend bool operator > (const MyString leftOp, const MyString rightOp);
        friend bool operator >= (const MyString leftOp, const MyString rightOp);
        friend bool operator == (const MyString leftOp, const MyString rightOp);
        friend bool operator != (const MyString leftOp, const MyString rightOp);
        
        MyString operator = (const MyString &right); // for client code

        // don't use strncpy,strncat,strncmp or anything from string class
        void readline(std::istream &in, char delimiter);
        // 19 functions including friends
        int length() const;
    private:
        int length(const char* myArray) const;
        char *charArray;
    };
}

#include "mystring.h"
#include <cstring>
using namespace std;

namespace cs_mystring
{
    MyString::MyString()
    {
        charArray = new char[5];
    }

    MyString::MyString(const char *inString)
    {
        //delete[] charArray;
        charArray = new char[strlen(inString) + 1];

       // length(inString);
        strcpy(charArray, inString);
    }

    MyString::MyString(const MyString& copyMe)
    {
        *this = MyString(copyMe.charArray);
    }

    MyString::~MyString()
    {
        delete[] charArray;
        charArray = nullptr;
    }

    MyString MyString::operator+=(const MyString rightOp)
    {
        return MyString();
    }

    char MyString::operator[](int index) const
    {
        return 0;
    }

    char& MyString::operator[](int index)
    {
        char temp;
        return temp;
    }


    //friend std::ostream& operator << (std::ostream& out, const MyString& printMe);
    std::ostream &operator << (std::ostream &out, const MyString &printMe)
    {
        out << printMe.charArray;

        return out;
    }

    istream &operator >> (istream &in, MyString &newString)
    {
        while (isspace(in.peek()))
        {
            in.ignore();
        }

        char temp[MAX_INPUT_SIZE + 1];
        in.getline(temp, MAX_INPUT_SIZE, ' ');
        //delete[] newString.charArray;
        newString = MyString(temp);

        return in;
    }

    MyString operator+(const MyString leftOp, const MyString rightOp)
    {
        return MyString();
    }

    bool operator<(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    bool operator<=(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    bool operator>(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    bool operator>=(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    bool operator==(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    bool operator!=(const MyString leftOp, const MyString rightOp)
    {
        return false;
    }

    MyString MyString::operator=(const MyString& right)
    {
        return MyString(right.charArray);
    }

    void MyString::readline(std::istream& in, char delimiter)
    {
        getline(in, charArray, delimiter)
    }

    int MyString::length() const
    {
        return length(charArray);
    }

    int MyString::length(const char* myArray) const
    {
        int length = 0;
        char* temp = charArray;
        while (temp)
        {
            length++;
            temp++;
        }
        return length;
    }
}

[jetm0t0]

#include "mystring.h"
#include <fstream>
#include <cctype>      // for toupper()
#include <string>
#include <cassert>
#include <iostream>
using namespace std;
using namespace cs_mystring;

void BasicTest();
void RelationTest();
void ConcatTest();
void CopyTest();
MyString AppendTest(const MyString& ref, MyString val);
string boolString(bool convertMe);

int main()
{
    cout << "Project#4: MyString class (ADT) client program to test CLASS INVARIANT\n";
    cout << "The class has one private data member defined as follows:\n";
    cout << "char *str;\n";
    cout << "str always represents a valid null-terminated c-string for each instance\n";
    cout << "of a MyString class object class.\n";
    cout << "The client code provides an interface to test correct operations on MyString\n";
    cout << "objects\n";

    cout << "\nHere is a list of tests and the order in which the tests will be conducted\n";
    cout << "1. BasicTest\n";
    cout << "2. RelationTest\n";
    cout << "3. ConcatTest\n";
    cout << "4. CopyTest\n\n";

    BasicTest();
  /*  RelationTest();
    ConcatTest();
    CopyTest();

    cout << "\nProject#4 -MyString class (ADT) testing now concluded.\n";
    cout << "MyString class destructor has been invoked and memory from the heap deallocated\n";
    cout << "Check output for correct results from the MyString class (ADT) implementation.\n";
    */

    return 0;
}


string boolString(bool convertMe) {
    if (convertMe) {
        return "true";
    }
    else {
        return "false";
    }
}


void BasicTest()
{
    MyString s;
    int stringLength;

    cout << "***********************************************************************\n";
    cout << "* Basic Test: Testing various member constructor options and nonmember*\n";
    cout << "* friend ostream << operator for basic MyString object creation &     *\n";
    cout << "* printing                                                            *\n";
    cout << "***********************************************************************\n";

    const MyString strs[] =
    { MyString("Wow"), MyString("C++ is neat!"),
     MyString(""), MyString("a-z") };


    for (int i = 0; i < 4; i++) {
        cout << "string [" << i << "] = " << strs[i] << endl;
    }

    cout << "\n***********************************************************************\n";
    cout << "* Basic Test: Testing nonmember friend istream >> and ostream <<      *\n";
    cout << "* operators for reading and display of MyString objects from data file*\n";
    cout << "***********************************************************************\n";

    cout << endl << "----- first, word by word" << endl;
    ifstream in("mystring_data.txt");
    assert(in);
    while (in.peek() == '#') {
        in.ignore(128, '\n');
    }
    in >> s;
    while (in) {
        cout << "Read string = " << s << endl;
        in >> s;
    }
    in.close();

    cout << "\n*************************************************************************\n";
    cout << "* Basic Test: Testing member function readline read, a nonmember friend *\n";
    cout << "* istream >> and ostream <<operators for reading and display of MyString*\n";
    cout << "* objects from data file                                                *\n";
    cout << "* ***********************************************************************\n";

    cout << endl << "----- now, line by line" << endl;
    ifstream in2("mystring_data.txt");
    assert(in2);
    while (in2.peek() == '#') {
        in2.ignore(128, '\n');
    }
    s.readline(in2, '\n');
    while (in2) {
        cout << "Read string = " << s << endl;
        s.readline(in2, '\n');
    }
    in2.close();

    cout << "\n****************************************************************************\n";
    cout << "* Basic Test: Testing access to characters (using const and non const)     *\n";
    cout << "* using constructors, member function length that returns a cstring length,*\n";
    cout << "* the square bracket [] overloaded operator and a nonmember friend         *\n";
    cout << "* ostream << for display of MyString objects content                       *\n";
    cout << "* **************************************************************************\n";

    cout << endl << "----- Testing access to characters (using const)" << endl;
    const MyString s1("abcdefghijklmnopqsrtuvwxyz");
    cout << "Whole string is " << s1 << endl;
    cout << "now char by char: ";
    stringLength = s1.length();
    for (int i = 0; i < stringLength; i++) {
        cout << s1[i];
    }


    cout << endl << "\n----- Testing access to characters (using non-const)" << endl;
    MyString s2("abcdefghijklmnopqsrtuvwxyz");
    cout << "Start with " << s2;
    stringLength = s2.length();
    for (int i = 0; i < stringLength; i++) {
        s2[i] = toupper(s2[i]);
    }
    cout << " and convert to " << s2 << endl;
}

My call and issue for the readline function with main is starts at:
where it says " in >> s; "

cout << "* Basic Test: Testing nonmember friend istream >> and ostream <<      *\n";
    cout << "* operators for reading and display of MyString objects from data file*\n";
    cout << "***********************************************************************\n";

    cout << endl << "----- first, word by word" << endl;
    ifstream in("mystring_data.txt");
    assert(in);
    while (in.peek() == '#') {
        in.ignore(128, '\n');
    }
    in >> s; 

I also chopped off main after RelationTest since main is too big

[jetm0t0]

Here's the read file saved as code, since IDK how to preserve the spaces:

# This file has some strings that are used in the string test to check
# reading strings from files. The default overloaded >> of your string
# class should skip over any leading spaces and read characters into
# the string object, stopping at the first whitespace character (this is
# similar to the behavior of >> on char *).  The read method of the
# string class is a little fancier. It allows client to restrict
# how many characters at max to read and what character to use as
# delimiter, so you can stop at newline instead of space, for example.
# Reading consumes the delimiting character, so the next read starts
# after that.
#
The  first  time  we  will
    read individual words, next
we read whole lines

[kbw]

I am having a hard time properly writing a readLine function

    void MyString::readline(std::istream& in, char delimiter)
    {
        getline(in, charArray, delimiter)
    }

A std::istream is a base class that represents some abstract input. It's always something more physical, like a stream in memory (std::istringstream), or a file (std::ifstream), ... or some custom stream. So using getline() as it's implementation is wrong, You need to read one byte at a time until you hit your delimiter.

It might be useful for readLine() to return the stream, so it can be used in a conditional statement.

I also have a bit more overload functions to write, <, > <= , >=, [],

        friend MyString operator + (const MyString leftOp, const MyString rightOp);
        MyString operator += (const MyString rightOp);

        friend bool operator < (const MyString leftOp, const MyString rightOp);
        friend bool operator <= (const MyString leftOp, const MyString rightOp);
        friend bool operator > (const MyString leftOp, const MyString rightOp);
        friend bool operator >= (const MyString leftOp, const MyString rightOp);
        friend bool operator == (const MyString leftOp, const MyString rightOp);
        friend bool operator != (const MyString leftOp, const MyString rightOp);

The parameters should be passed by const reference, rather than by value.


Also, you need to think about initialization.
1. Why does the default constructor allocate an array of 5 chars?
2. How does your class remember the length of it's content?
3. You don't seem to allow '\0' in your string. Why not?
4. You seemed to have realized there's a problem with the copy constructor, but not sure how to fix it.
5. You should implement move constructor/operator too.


Also, step thru your code with a debugger at least once, so you can see what you've done. The copy in those value parameters would have shown up as something to look into.

[jetm0t0]

Ok ya, I got a tutor to help with my readline function. I'm pretty sure that's working. The operator functions like <, > were given to me by my teacher...
I made the constructor 5 just to give it a low starting number, just to initialize it to something.
Everything that's written in main() was not written by me and I'm not allowed to change it.
I think I got my copy constructor to work, I needed to check for self assignment right?
why would I need the move constructor? It's not mentioned at all in my instructions.

Where I'm at now I just downloaded and properly installed VLD(visual leak detector) for visual studio, and I ran it and I don't think it found any memory leaks, but my tutor said I may have some arise in places where I delete charArray. I'm getting exception thrown errors, and access violation at location ..... from maybe where any of my delete[] charArray is at
Most specifically at my readline function with the line:

charArray[index] = next;

[kbw]

I think I got my copy constructor to work, I needed to check for self assignment right?

No, not really. Self assignment is a concern for the assignment operator. However the constructor is called when an instance is created.

Let's look are your code:

    MyString::MyString()
    {
        charArray = new char[5];
    }

    MyString::MyString(const char *inString)
    {
        //delete[] charArray;
        charArray = new char[strlen(inString) + 1];

       // length(inString);
        strcpy(charArray, inString);
    }

    MyString::MyString(const MyString& copyMe)
    {
        *this = MyString(copyMe.charArray);
    }

    MyString MyString::operator=(const MyString& right)
    {
        return MyString(right.charArray);
    }

The default constructor creates an array of length 5. But there's no way for you to tell that the array length is 5, because you haven't saved and you haven't placed anything in the buffer, so you can't check for a null terminator.

The copy constructor is broken. It should copy the content, which in your case means:
1. allocating a block large enough to hold a copy of copyMe
2. copy the data into the block.

What it's doing is:
1. leave the buffer pointer uninitialized
2. call the assignment operator
3. Create a new temporary object, initialized from the buffer of the source
4. return a reference to that temporary object

What's wrong with that? Once operator= is done, it deletes that temporary, and you end up with *this = referencing something that's been deleted. We call this a dangling reference, it a serious error. And it doesn't show up as a memory leak. Further more, when the object is destroyed, it calls delete[] on a memory location that's already been deleted. It's really bad.

EDIT: Just noticed operator= returns an object by value. And that calls the copy constructor, which repeats the process. I expect this hangs, and if left running long enough, crashes the program (out of memory).

Also, it's easier to just add a member that tracks the length of the string, you might want to consider that.

[jetm0t0]

This is where I'm at now with copyMe, he had me temp comment out the delete in case there were leaks and wasn't sure we needed it:

    MyString::MyString(const MyString& copyMe)  
       : charArray (nullptr)
    {
        /*if (charArray != nullptr)
        {
            delete[] charArray;
            charArray = nullptr;
        }*/
        charArray = new char[strlen(copyMe.charArray) + 1]; 
        strcpy(charArray, copyMe.charArray); 
    }

so put in a count member in my class and I would have to update that everywhere I use charArray?

[seeplus]

Consider as one file:

#include <iostream>
#include <string>
#include <fstream>
#include <utility>

constexpr size_t MAX_INPUT_SIZE {127};

namespace cs_mystring {
	class MyString {
	public:
		MyString();
		MyString(const char* inString);
		MyString(const MyString& copyMe);
		MyString(MyString&& moveMe) noexcept;
		~MyString();

		MyString& operator+= (const MyString& rightOp);
		char operator[](size_t index) const;
		char& operator[](size_t index);
		MyString& operator=(const MyString right) noexcept;
		std::istream& readline(std::istream& in, char delimiter = '\n');
		size_t length() const;

		friend std::ostream& operator << (std::ostream& out, const MyString& printMe);
		friend std::istream& operator >> (std::istream& in, MyString& readMe);
		friend MyString operator+(const MyString& leftOp, const MyString& rightOp);
		friend bool operator <(const MyString& leftOp, const MyString& rightOp);
		friend bool operator <=(const MyString& leftOp, const MyString& rightOp);
		friend bool operator >(const MyString& leftOp, const MyString& rightOp);
		friend bool operator >=(const MyString& leftOp, const MyString& rightOp);
		friend bool operator ==(const MyString& leftOp, const MyString& rightOp);
		friend bool operator !=(const MyString& leftOp, const MyString& rightOp);

	private:
		size_t length(const char* myArray) const;
		char* charArray {};
	};
}

namespace cs_mystring {
	MyString::MyString() : charArray {new char[5]} { }

	MyString::MyString(const char* inString) : charArray {new char[strlen(inString) + 1]} {
		strcpy(charArray, inString);
	}

	MyString::MyString(const MyString& copyMe) : MyString(copyMe.charArray) {}

	MyString::MyString(MyString&& moveMe) noexcept {
		std::swap(moveMe.charArray, charArray);
	}

	MyString::~MyString() {
		delete[] charArray;
	}

	MyString& MyString::operator+=(const MyString& rightOp) {
		return *this;
	}

	char MyString::operator[](size_t index) const {
		return charArray[index];
	}

	char& MyString::operator[](size_t index) {
		return charArray[index];
	}

	std::ostream& operator<<(std::ostream& out, const MyString& printMe) {
		return out << printMe.charArray;
	}

	std::istream& operator>>(std::istream& in, MyString& newString) {
		while (std::isspace(in.peek()))
			in.ignore();

		char temp[MAX_INPUT_SIZE + 1] {};
		size_t cnt {};

		for (; cnt < MAX_INPUT_SIZE && !std::isspace(in.peek()) && in.get(temp[cnt]); ++cnt);
		temp[cnt] = 0;
		newString = MyString(temp);
		return in;
	}

	MyString operator+(const MyString& leftOp, const MyString& rightOp) {
		return MyString();
	}

	bool operator<(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	bool operator<=(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	bool operator>(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	bool operator>=(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	bool operator==(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	bool operator!=(const MyString& leftOp, const MyString& rightOp) {
		return false;
	}

	MyString& MyString::operator=(MyString right) noexcept {
		std::swap(right.charArray, charArray);
		return *this;
	}

	std::istream& MyString::readline(std::istream& in, char delimiter) {
		char temp[MAX_INPUT_SIZE + 1] {};

		in.getline(temp, MAX_INPUT_SIZE, delimiter);
		*this = MyString(temp);

		return in;
	}

	size_t MyString::length() const {
		return length(charArray);
	}

	size_t MyString::length(const char* myArray) const {
		auto s {myArray};

		for (; *s; ++s);
		return s - myArray;
	}
}

std::string boolString(bool convertMe) {
	if (convertMe) {
		return "true";
	} else {
		return "false";
	}
}

void BasicTest() {
	using namespace cs_mystring;

	std::cout << "***********************************************************************\n";
	std::cout << "* Basic Test: Testing various member constructor options and nonmember*\n";
	std::cout << "* friend ostream << operator for basic MyString object creation &     *\n";
	std::cout << "* printing                                                            *\n";
	std::cout << "***********************************************************************\n";

	const MyString strs[] {MyString("Wow"), MyString("C++ is neat!"),  MyString(""), MyString("a-z")};

	for (size_t i {}; i < 4; ++i)
		std::cout << "string [" << i << "] = " << strs[i] << '\n';

	std::cout << "\n***********************************************************************\n";
	std::cout << "* Basic Test: Testing nonmember friend istream >> and ostream <<      *\n";
	std::cout << "* operators for reading and display of MyString objects from data file*\n";
	std::cout << "***********************************************************************\n";

	std::cout << "\n----- first, word by word\n";

	std::ifstream in("mystring_data.txt");
	//assert(in);

	while (in.peek() == '#')
		in.ignore(128, '\n');

	for (MyString s; in >> s; )
		std::cout << "Read string = " << s << '\n';

	in.close();

	std::cout << "\n*************************************************************************\n";
	std::cout << "* Basic Test: Testing member function readline read, a nonmember friend *\n";
	std::cout << "* istream >> and ostream <<operators for reading and display of MyString*\n";
	std::cout << "* objects from data file                                                *\n";
	std::cout << "* ***********************************************************************\n";

	std::cout << "\n----- now, line by line\n";

	std::ifstream in2("mystring_data.txt");
	//assert(in2);

	while (in2.peek() == '#')
		in2.ignore(128, '\n');

	for (MyString s; s.readline(in2); )
		std::cout << "Read string = " << s << '\n';

	in2.close();

	std::cout << "\n****************************************************************************\n";
	std::cout << "* Basic Test: Testing access to characters (using const and non const)     *\n";
	std::cout << "* using constructors, member function length that returns a cstring length,*\n";
	std::cout << "* the square bracket [] overloaded operator and a nonmember friend         *\n";
	std::cout << "* ostream << for display of MyString objects content                       *\n";
	std::cout << "* **************************************************************************\n";

	std::cout <<  "\n----- Testing access to characters (using const)\n";

	const MyString s1("abcdefghijklmnopqsrtuvwxyz");
	const auto stringLength1 {s1.length()};

	std::cout << "Whole string is " << s1 << '\n';
	std::cout << "now char by char: ";

	for (size_t i {}; i < stringLength1; ++i)
		std::cout << s1[i];

	std::cout << "\n\n----- Testing access to characters (using non-const)\n";

	MyString s2("abcdefghijklmnopqsrtuvwxyz");
	const auto stringLength2 {s2.length()};

	std::cout << "Start with " << s2 << '\n';

	for (size_t i {}; i < stringLength2; ++i)
		s2[i] = toupper(s2[i]);

	std::cout << " and convert to " << s2 << '\n';
}

int main() {
	BasicTest();
}

[jetm0t0]

I think I understand what you are saying this code:

charArray = new char[strlen(copyMe.charArray) + 1];

won't work because it's using assignment? and then it would be easier if I set up the new array with a length the size of some counter member? in that case would I even ever use "="? because I could just set up count1 as a member and:

count1 = 5;
new char[strlen(copyMe.charArray) + count1];
strcpy(charArray, copyMe.charArray + count1); 

?

[jetm0t0]

Got some help, turns out my major issue, my program was just hanging, after main tries to read char by char. I had some loops not needed in my length function.

    int MyString::length(const char* myArray) const
    {
        int length = 0;
        char* temp = charArray;
        while (temp)
        {
            length++;
            temp++;
        }
        return length;
    }
}

this should just be

    int MyString::length(const char* myArray) const
    {
        return strlen(charArray);
    }

[jetm0t0]

Damn didn't notice one line had garbage during the concat test, oh well. I can look at it later, I already turned it in.

[seeplus]

I didn't use strlen() in my code above as I thought you weren't to use the cstring functions. However from that code you can see how to code strlen().

If you post your revised code, we could advise re the concat garbage.

[jetm0t0]

This was his instructions on that: The C++ library has numerous functions for handling C-strings. These functions (such as strlen, strcpy, strcmp and cin.getline) perform various tests and manipulations, and require the <cstring> header file be included.
Unfortunately right now I don't have time to find the culprit of the garbage, a really good tutor also pointed out a mem leak of 71 bytes, but I will review the answer key when I get it.

[deleted account xyzzy]

The C++ standard library copied over the C library for the most part, including the C functions for handing C strings. strlen, strcopy, strcmp are functions in the C library.