HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Program to reverse a string using std::string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stdreverseprogramusingstring

Problem

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
#include
#include

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& displayString(const stringOps &a);

        //3. Reverse the string and return obj of type stringOps.
        stringOps reverseStr(stringOps &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& stringOps::displayString(const stringOps &a)
{
//cout << a.arr << endl;
return a.arr;
}

//3. reverse array of the object.
stringOps stringOps::reverseStr(stringOps &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:

Solution

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 
#include 


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&. If you want to move from it, the type should be std::string&&.

~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& displayString(const stringOps &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 &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.

Code Snippets

#include <iostream>
#include <cstdio>
using namespace std;
class stringOps
string arr;
int length;

Context

StackExchange Code Review Q#135123, answer score: 4

Revisions (0)

No revisions yet.