Program to reverse a string using std::string Background: I'm trying to learn C++ and I'm doing that by writing some programs by myself. I'm decent with C, but I'm having trouble translating to OOP paradigm. I'm reading from learncpp.com and a book called Thinking in C++.

I'm noticing that reading doesn't really help much as much as writing programs and running into issues and solving them.

Here is a program I've written to reverse a string in C++ using std::string (SO tells me this should be used as opposed to C-style strings/char arrays):

example_one.h

#ifndef example_one_h
#define example_one_h

#include&lt;iostream&gt;
#include&lt;string&gt;
#include&lt;cstdio&gt;

using namespace std;

class stringOps{

    private:
        string arr;
        int length;

    public:
        // Constructor - Destructor
        stringOps(string arr);
        ~stringOps();
        // Member functions

        //1. get length of string
        int getLength();

        //2. print string
        const string&amp; displayString(const stringOps &amp;a);

        //3. Reverse the string and return obj of type stringOps.
        stringOps reverseStr(stringOps &amp;a);

};

#endif


example_one.cc

#include "example_one.h"

/* Constructor and Destructor */
stringOps::stringOps(string a):arr(a), length(a.size()) {
}

stringOps::~stringOps() {
}

/* Member functions */
// 1.  get Length
int stringOps::getLength()
{
    return arr.size();
}

//2. display string
const string&amp; stringOps::displayString(const stringOps &amp;a)
{
    //cout &lt;&lt; a.arr &lt;&lt; endl;
    return a.arr;
}

//3. reverse array of the object.
stringOps stringOps::reverseStr(stringOps &amp;a)
{
    reverse(a.arr.begin(), a.arr.end());
    return a;
}


/*  -------------------------------- Main  --------------------- */
int main()
{
    stringOps strA("Hello");

    string x = strA.displayString(strA);
    printf("Original string: %s \n", x.c_str());

    int len = strA.getLength();
    printf("String length:%d \n",(len));

    strA.reverseStr(strA);
    string y = strA.displayString(strA);
    printf("Reverse string: %s \n", y.c_str());



    return 0;
}


Questions:


What is good about this code (so that I can continue to do this for other programs)?
What is bad about this code? (Be brutal in your assessment.) Specifically, I have doubts about the best approach to take while writing functions 2 and 3.

Function 2:

const string&amp; stringOps::displayString(const stringOps &amp;a)
{
    //cout &lt;&lt; a.arr &lt;&lt; endl;
    return a.arr;
}


I tried doing this with return type as void and cout within the function, but apparently there's going to be an error for this with iostream. So is returning const string&amp; a good way to do this? I passed by const reference because I do not change the object within the function.

Function 3:

stringOps stringOps::reverseStr(stringOps &amp;a)
{
    reverse(a.arr.begin(), a.arr.end());
    return a;
}



This again takes the argument of type stringOps object by reference and directly manipulates the string. However, if I were to pass this argument as a const, how do I go about this?
Motivation for me to do so is that I want to return a manipulated string that is new, and not manipulate the object that is coming in directly.


The header would look something like this:

string stringOps::reverseStr(const stringOps &amp;a)


Do I declare static string inside the function and return it? I tried using a local non-static string (which would be destroyed outside the function, but still for experiment purposes) and I got an error that said:


  read-only variable is not assignable


I'm assuming this is because it's a const reference.
The functionality is already present in the standard library. Your class has very limited usability and I would prefer simple std::reverse(str.begin(), str.end()). But lets put it aside and deal with the code you've written.

#include &lt;iostream&gt;
#include &lt;cstdio&gt;


Very bad idea to mix these two. I/O may become unordered.

using namespace std;


Why is “using namespace std” in C++ considered bad practice?.

class stringOps


Operations on class's data should be part of it, so naming it stringOps is not good idea. C++ programmers often use operator overloading to expand on some class.

You begin your class definition by private keyword, but everything in class is private by default, whereas in struct it is public. It is the only difference between the two.

    string arr;


What arr means? Nothing comes to mind. You should use something like str. It is good to give your variables good name. It greatly increases readability of the code.

   int length;


Declared, but never used.

    stringOps(string arr);


If you want to make a copy of the string type should be const std::string&amp;. If you want to move from it, the type should be std::string&amp;&amp;. 

    ~stringOps();


You declared destructor that does nothing. Compiler generates this by default if you don't provide any destructor. Remove it.

    int getLength();


From return type (int) I may suspect that the code can return negative size. It should be std::size_t, which is meant to be used in context of size. Further reading: When should I use std::size_t?

The functionality is present in std::string. On top of that, you use arr.length() as return value, not length.

    const string&amp; displayString(const stringOps &amp;a);


Why would you return const reference to it? I think users expect a copy of internal string. All operations on copying are noexcept, not including memory allocation, of course.

The next thing is arguments. It would be better to just omit argument, and return internal string.

stringOps reverseStr(stringOps &amp;a);


Again, argument should be omitted. The internal string should be reversed and returned. Also, it would be better to return std::string.

Answer for your first question:

Don't do it in the future. It's not that your code is wrong. The purpose of it is very wrong.