Custom string class problems

[numba1sonman]

Hello,
for one of my college projects I am asked to create a basic string class to call upon in my code. I feel like I am very close, but something is not working correctly.

  strcpy(temp, strCost.c_str());
	str = strtok(temp, ".");

The c_str() is what is flagging an error. It says that in class "Mystring" there is no member for "c_str".
Which means I need to have it in my class to function, I am just unsure what to change to have it work correctly.

This is my string class that I am using.

class Mystring {

   
    char* str;

public:
    
    Mystring();

    
    Mystring(char* val);

    
    Mystring(const Mystring& source);

    
    Mystring(Mystring&& source);

   
    ~Mystring() { delete str; }

    friend class Potion;
};


Mystring::Mystring()
    : str{ nullptr }
{
    str = new char[1];
    str[0] = '\0';
}


Mystring::Mystring(char* val)
{
    if (val == nullptr) {
        str = new char[1];
        str[0] = '\0';
    }

    else {

        str = new char[strlen(val) + 1];

        
        strcpy(str, val);
        str[strlen(val)] = '\0';

        cout << "The string passed is: "
            << str << endl;
    }
}


Mystring::Mystring(const Mystring& source)
{
    str = new char[strlen(source.str) + 1];
    strcpy(str, source.str);
    str[strlen(source.str)] = '\0';
}


Mystring::Mystring(Mystring&& source)
{
    str = source.str;
    source.str = nullptr;
}

On a side note, I also noticed that removing the "Move constructor &&" from the code seemed to fix a lot of errors.
Is a move constructor necessary, I do not understand its purpose.

[Ganado]

Where do you declare or initialize the strCost variable?

For std::strings, the .c_str() function is used for compatibility with C-style interfaces that expect a const char* as a parameter.

So it would look something like

const char* Mystring::c_str() const
{
    return str;
}

Learning about move constructors is a topic all on its own.
https://medium.com/@seanoughton/c-move-constructors-97594b8af0b1
https://www.learncpp.com/cpp-tutorial/move-constructors-and-move-assignment/

What is the error you run into? The purpose of a move constructor is to extract the contents of the mystring&& object into the object being created. It repurposes the pointer instead of copying the contents of what the pointer points to, thereby being more efficient. The mystring&& object should no longer be used afterwards, and will be destroyed when it goes out of scope.

PS: I don't think line 59 is strictly necessary. According to strcpy documentation, the null terminator is copied over. https://stackoverflow.com/questions/11830979/c-strcpy-function-copies-null
But it doesn't hurt.

PPS: I can almost guarantee you that it's bad practice for your generic Mystring class to have to be 'friends' with a specific class like Potion. You shouldn't have to do something like that. You should let the necessary functionality of your string be exposed through functions, just like how std::strings don't need to be friends with a Potion to be useful.

[seeplus]

The destructor needs [] for delete. Also note that move constructor (and move assignment) should be marked as noexcept.

Consider:

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

class Mystring {
	char* str {};

public:
	Mystring();
	Mystring(const char* val);
	Mystring(const Mystring& source);
	Mystring(Mystring&& source) noexcept;
	~Mystring() { delete [] str; }

	Mystring& operator=(const Mystring&) = delete;
	Mystring& operator=(Mystring&&) = delete;

	friend std::ostream& operator<<(std::ostream& os, const Mystring& ms) {
		return os << (ms.str != nullptr ? ms.str : "");
	}
};

Mystring::Mystring() : str {new char[1] {}} {};

Mystring::Mystring(const char* val) {
	if (val == nullptr)
		str = new char[1] {};
	else {
		str = new char[strlen(val) + 1];
		strcpy(str, val);

		//std:: cout << "The string passed is: " << str << '\n';
	}
}

Mystring::Mystring(const Mystring& source) : str {new char[strlen(source.str) + 1]} {
	strcpy(str, source.str);
}

Mystring::Mystring(Mystring&& source) noexcept {
	std::swap(source.str, str);
}

int main() {
	Mystring s1;

	std::cout << s1 << '\n';

	Mystring s2 {"foobar"};

	std::cout << s2 << '\n';

	Mystring s3 {s2};

	std::cout << s3 << '\n';

	Mystring s4 {std::move(s3)};

	std::cout << s4 << "  " << s3 << '\n';
}

[numba1sonman]

What is the noexcept? I have not used it yet.

I will work on it so it is not friended with the other class too. Thank you for the help so far everyone.

I will post my resolved code when I get it working :)

[numba1sonman]

I am getting less errors, however all of my "=" do not like something.


it says " Mystring &Mystring::operator=(const Mystring &) +1 overload

Function "MyString::Operator=(const Mystring &)" (declared at line 31 of string.h) cannot be referenced -- it is a deleted function.

[coder777]

For noexcept see:

https://en.cppreference.com/w/cpp/language/noexcept

Function "MyString::Operator=(const Mystring &)" (declared at line 31 of string.h) cannot be referenced -- it is a deleted function.

When you want the copy/move operator=() you need to remove the = delete and implement them (which is similar to the respective constructors).

[seeplus]

The default operator()= is not suitable when dynamic memory is used (as here). Hence the =delete to prevent run-time issues. If = is required, then a proper implementation for the operator=() function needs to be implemented.

This is quite easy to implement. In my code above replace L15-16 with:

Mystring& operator=(Mystring rhs) noexcept {
    std::swap(str, rhs.str);
    return *this;
}

You'll need #include <utilities> for the std::swap()

[numba1sonman]

Thank you for the resource, I will read up on noexcept and try to become more familiar with the operators. I do not fully understand your explanations, but I am fairly new to constructors/destructors and classes. So reading up on those will do me good.

[numba1sonman]

Got it working correctly I think. At least the output is what I want.

I was receiving LINK 2005 errors, so I tried placing all of MyString functions inside the class instead of defining them outside of it. That seemed to work.
I also needed to place my char * str in public and not private so everything could access it.

class Mystring 
{
public:

		// Initialise the char array
	char* str;
  
		// Function to illustrate Constructor
		// with no arguments
	Mystring() : str{ new char[1] {} } {};

		// Function to illustrate Constructor
		// with one arguments
	Mystring(const char* val)
	{
		if (val == nullptr)
			str = new char[1]{};
		else {
			str = new char[strlen(val) + 1];
			strcpy(str, val);

			//std:: cout << "The string passed is: " << str << '\n';
		}
	}

		// Function to illustrate
		// Copy Constructor
	Mystring(const Mystring& source) : str{ new char[strlen(source.str) + 1] }
	{
		strcpy(str, source.str);
	}

		// Function to illustrate
		// Move Constructor
	Mystring(Mystring&& source)noexcept
	{
		std::swap(source.str, str);
	}

		// Destructor
	~Mystring() { delete[] str; }

	Mystring& operator=(Mystring rhs) {
		std::swap(str, rhs.str);
		return *this;
	}

	friend std::ostream& operator<<(std::ostream& os, const Mystring& ms) {
		return os << (ms.str != nullptr ? ms.str : "");
	}

};

I do not know if this is the best practice. Nothing in private and defining inside the class. But its working?

Thank you all!

[jonnin]

heh we just had a discussion about that. Making stuff private just so you have to write getters and setters .. be practical was what most people said. Purism would demand that you do that (private with accessors) but purist code isnt practical for all things. Making the variable public is fine if you are willing to own the consequences (mostly, a user fooling with the variable directly in bad ways, eg setting it to nullptr and leaking memory that should have been deleted first for a simple example).

and, also, depends on if this is homework (what is your professor going to want) or learning (did you learn what you wanted from it) or whatever? I am going out on a limb and assuming its not useful (c++ has a better string built into it, no offense :) )

[seeplus]

But its working?

With what code as main() ? why does it need str public?

[numba1sonman]

Yeah, the code is mostly for learning how to build out classes and create objects.

Here was my main code that needed the str public.

int coin = 0;
	int currencyCount = 0;
	float converstionRate = 0.0f;
	char temp[300];
	Mystring strCost;

	strCost = curPotion.getCost();

	strcpy(temp, strCost.str);
	str = strtok(temp, ".");

Because my strCost was calling upon my custom string, it needed the .str from the file.

[AbstractionAnon]

I'm going to take the purist approach here and say that there is no reason that str needs to be public. Making it public violates the encapsulation objective of C++.

Line 10 causes a memory leak. Line 5 does a new. Line 10 wipes out the pointer to the memory allocated at line 5. This is exactly the reason that str should not be public.

[seeplus]

If you needed access to the str as a type const char* to pass as a pointer to another function (eg strcpy), then the way to do this is to provide a custom type overload within the class (or as a separate function as per Ganado's post above):

#include <iostream>

class Mystring {
	char* str {};

public:
	Mystring();
	Mystring(const char* val);
	Mystring(const Mystring& source);
	Mystring(Mystring&& source) noexcept;
	~Mystring() { delete[] str; }

	Mystring& operator=(Mystring rhs) noexcept;
	operator const char* () const noexcept;

	friend std::ostream& operator<<(std::ostream& os, const Mystring& ms) {
		return os << (ms.str != nullptr ? ms.str : "");
	}
};

Mystring::Mystring() : str {new char[1] {}} {};

Mystring::Mystring(const char* val) {
	if (val == nullptr)
		str = new char[1] {};
	else {
		str = new char[strlen(val) + 1];
		strcpy(str, val);
	}
}

Mystring::Mystring(const Mystring& source) : str {new char[strlen(source.str) + 1]} {
	strcpy(str, source.str);
}

Mystring::Mystring(Mystring&& source) noexcept {
	std::swap(source.str, str);
}

Mystring& Mystring::operator=(Mystring rhs) noexcept {
	std::swap(str, rhs.str);
	return *this;
}

Mystring::operator const char* () const noexcept {
	return static_cast<const char*>(str);
}

int main() {
	char temp[300] {};
	const Mystring strCost {"123.456"};

	strcpy(temp, strCost);

	auto str {strtok(temp, ".")};

	std::cout << str << "  ";
	std::cout << strtok(nullptr, ".");
}

123  456

It can now also be done using just Mystring. Possibly something like:

int main() {
	const Mystring strCost {"123.456"};
	auto temp {strCost};
	auto str {strtok(const_cast<char*>(static_cast<const char*>(temp)), ".")};

	std::cout << str << "  ";
	std::cout << strtok(nullptr, ".");
}

[Peter87]

Rather than having Mystring convert to const char* implicitly I think it would be better to use a member function. That's how std::string works. There you call c_str() to get a const char*.