4
#include <iostream>
#include <string>
using namespace std;
int main()
{
  string s1;
  cin>>s1;

  int n=s1.size();
  string s2;
  for(int a=0;a<n;a++)
  {
    s2[a]=s1[n-1-a];        
  }
  cout<<s2;
}

However I am not getting any output, But I can print elements of reversed string. Can anybody please help.

BoBTFish
  • 19,167
  • 3
  • 49
  • 76
kp'
  • 43
  • 4
  • @陳力 This isn't a duplicate, since it isn't really a question about a reversal algorithm. The asker doesn't know it, but it's really a question about allocating space within a `string`. – BoBTFish May 23 '18 at 06:45
  • @BoBTFish Ok, deleted. – Chen Li May 23 '18 at 06:48
  • http://en.cppreference.com/w/cpp/algorithm/reverse – hellow May 23 '18 at 06:57
  • I'd like to stress again, this is **not** a duplicate of https://stackoverflow.com/questions/198199 - that question is specifically about reversal **in place**. This question is clearly using a new string to store the result, and is only superficailly about reversal. (It may be a duplicate of some other question about writing to string elements that don't exist, but I am not aware of a good duplicate for that). – BoBTFish May 23 '18 at 07:08
  • You might want to think about changing the title of the question, because your *problem* isn't about string reversal... – DevSolar May 23 '18 at 07:32
  • I edited the title - I think this best summarises the problem. **Now we know the answer**, it is clear the reversal is irrelevant, but the asker didn't know that, so I think this is a good summary of the **question** (without capturing the answer). – BoBTFish May 23 '18 at 07:39

4 Answers4

6
string s2; // no size allocated
for(int a=0;a<n;a++)
{
    s2[a]=s1[n-1-a]; // write to somewhere in s2 that doesn't exist yet

You are writing into elements of s2 that you never created. This is Undefined Behaviour, and anything may happen, including appearing to work normally. In this case, you are probably overwriting some random place in memory. It might crash, or that memory might really exist but not break anything else right away. You might even be able to read that back later, but it would only seem to work by pure accident.

You could catch this problem by always using s2.at(a) to access the data, as it will do a range check for you. ([] does not do a range check). There's a cost to this of course, and sometimes people will skip it in circumstances where they are certain the index cannot be out of bounds. That's debateable. In this case, even though you were probably sure you got the maths right, it still would have helped catch this bug.


You need to either create that space up front, i.e. by creating a string full of the right number of dummy values, or create the space for each element on demand with push_back. I'd probably go with the first option:

string s2(s1.size(), '\0'); // fill with the right number of NULs
for(int a=0;a<n;a++)
{
    s2.at(a)=s1.at(n-1-a); // overwrite the NULs

You might want to choose a printable dummy character that doesn't appear in your test data, for example '#', since then it becomes very visible when you print it out if you have failed to correctly overwrite some element. E.g. if you try to reverse "abc" but when you print it out you get "cb#" it would be obvious you have some off-by-one error.


The second option is a bit more expensive since it might have to do several allocations and copies as the string grows, but here's how it would look:

string s2; // no space allocated yet
for(int a=0;a<n;a++)
{
    s2.push_back(s1.at(n-1-a)); // create space on demand

I assume you are doing this as a learning exercise, but I would recommend against writing your own reverse, since the language provides it in the library.

BoBTFish
  • 19,167
  • 3
  • 49
  • 76
  • as u said i m writting into elements of s2 that were never created but i can access value of s2[0] or s2[1] – kp' May 23 '18 at 06:47
  • 2
    Yes, and this is Undefined Behaviour. By the standard, it can do anything, including getting a seemingly correct output or crashing the whole system. If you would use `at()` method, you'd get an exception. – Yksisarvinen May 23 '18 at 06:53
  • 1
    @kp' Exactly - you are not **allowed** to do this, but it might appear to work. I would guess the reason for this is the **small string optimisation**. For strings only a few characters long, most `std::string` implementations store the characters inside itself, rather than creating a buffer. This is **probably** how a default-constructed string is set up. So even though you haven't **in theory** created any space, **in practice** you **probably** won't actually crash because there is some space available. None of this is guaranteed though. – BoBTFish May 23 '18 at 06:53
4

You are utilizing something called Undefined Behaviour in your code. You try to access element of s2 at a position, but your string does not have that many chars (it's empty).

You can use std::string::push_back function to add a character on the last postion, so your code would look like this:

for(int a=0;a<n;a++)
{
    s2.push_back(s1[n-1-a]);        
}

EDIT, to address the other question, your console window probably closes before you can notice. That's why "you don't get any output".
Try this: How to stop C++ console application from exiting immediately?

Yksisarvinen
  • 18,008
  • 2
  • 24
  • 52
3

You can use the inbuilt reverse function in c++

#include<bits/stdc++.h>
using namespace std;
int main()
{
   string s1 = "string1";

   reverse(s1.begin(),s1.end());

   cout << s1;
   return 0;
}

Hope that helps :)

sjdm
  • 591
  • 4
  • 10
  • Yes I can but I want to know the reason why I m not getting output – kp' May 23 '18 at 06:36
  • @sjdm I'm not recommending it, because it's confusing, but you **can** leave `return 0;` out of `main()`, and it will be assumed that it is there as the last line. This is only true for `main()`. – BoBTFish May 23 '18 at 06:38
  • i added but that did not help – kp' May 23 '18 at 06:39
  • apologies I realised its because you have not initialised s2 correctly – sjdm May 23 '18 at 06:39
0

You can just construct the string with reverse iterators:

std::string reverse ( std::string const& in )
{
    return std::string { in.crbegin(), in.crend() };
}
KevinZ
  • 3,036
  • 1
  • 18
  • 26