patterncppMinor
Program to reverse a string using std::string
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
example_one.h
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:
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);
};
#endifexample_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
Very bad idea to mix these two. I/O may become unordered.
Why is “using namespace std” in C++ considered bad practice?.
Operations on class's data should be part of it, so naming it
You begin your class definition by
What
Declared, but never used.
If you want to make a copy of the string type should be
You declared destructor that does nothing. Compiler generates this by default if you don't provide any destructor. Remove it.
From return type (
The functionality is present in
Why would you return const reference to it? I think users expect a copy of internal string. All operations on copying are
The next thing is arguments. It would be better to just omit argument, and return internal string.
Again, argument should be omitted. The internal string should be reversed and returned. Also, it would be better to return
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.
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 stringOpsOperations 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 stringOpsstring arr;int length;Context
StackExchange Code Review Q#135123, answer score: 4
Revisions (0)
No revisions yet.