There are a lot of serious mistakes in your code, but first things first.
the core
The core of the application is to reverse letters in a std::string.
Now suppose I do this:
Code:
string sTheWord("Hello");
string sReversedWord;
The question is: would the code below work?
Code:
sReversedWord[0]=sTheWord[0];
The anwer is NO. Why?
Because C++ did NOT allocate any space for sReversedWord. So sReversedWord[0] is not defined yet (does not exist), since this is the first element of that word. But there IS NO element at all yet.
We can solve this problem by allocating some space for sReversedWord:
Code:
//Watch the space in the initialization!
sReversedWord(" ");
AHA! Now there exists a sReversedWord[0]! So this works now:
Code:
sReversedWord[0]=sTheWord[0];
cout<<sReversedWord<<endl;
//OUTPUT: t
But what if we need to copy more letters to sReversedWord? sReservedWord[1]..[n] does NOT exist! So do you want to allocate the number of spaces to sReservedWord, according to the number of elements in sTheWord? Offcourse not. That's why we need a better method: by using the operator+= (for example, see below for another way) :
Code:
string sTheWord("hi");
string sReversedWord; //Just a name for us, but not using space yet!
sReversedWord += sTheWord[1]; //AHA, now sReversedWord[0] is added on the spot!
sReversedWord += sTheWord[0]; //AHA, now sReversedWord[1] is added on the spot!
cout<<sReversedWord<<endl;
//OUTPUT: ih
Each time I add a letter to the whole std::string, C++ will allocate more memory for that extra element.
function handling
Your function handling is wrong. Look at this:
Code:
#include <iostream>
#include <cstdlib>
#include <string>
string reverse(word);
You are (forward) declaring a function. Good. But...
"word" has no type!
This is correct: string reverse(string word);
See? now the function knows what type the word is. Here is a little optimization though:
string reverse(const string& word);
If you don't know what this is then nevermind for now. Later we could chat about it huh?
function handling part 2
This is wrong what you did:
it should be: j = word.size();
See the "()"?
communication
Coding is communication I say. Make sure your variables and your function names
clearly tell strangers what you're up to. The difference between "word" and "wordr" is hard to spot, for example.
Take your time sir. Perhaps you won't be able to finish all your homework because you work neat, but really, Rambo's and C++ is not a good match. You'll find out soon enough when you move on with this.
the final program
Below is ANOTHER way to do the reversing. That makes three possibilities to reverse a std::string. In fact, you may think of 3 more ways. Or 4, or 5... :)
Code:
#include <iostream>
#include <string>
using namespace std;
//Optional functions: depends on IDE.
void wait_for_enter();
void reset_istream();
//Core
string reverse_stdString(const string&);
bool compare_stdStrings(const string&, const string&);
////////////////// int main //////////////////////////////
int main (int argc, char * const argv[])
{
string sWord, sReversedWord;
cout<<"Enter a word please: ";
//Robustness omitted for clarity.
cin>>sWord;
sReversedWord = reverse_stdString(sWord);
if( compare_stdStrings(sWord, sReversedWord) == true )
{
cout<<"Palindrome!"<<endl;
}
else
{
cout<<"Not a palindrome."<<endl;
}
reset_istream();
wait_for_enter();
return 0;
}
//////////////////// reverse the word ////////////////////////////
string reverse_stdString(const string& s)
{
string reversed;
int len = s.length();
while (len != 0)
{
//Remember, an array starts with index 0 and therefore ends with n-1 !!
reversed += s.substr(len-1, 1);
--len;
}
return reversed;
}
/////////check reversed word for equality with original word ///////
bool compare_stdStrings(const string& cmp1, const string& cmp2)
{
if(cmp1 == cmp2)
{
return true;
}
else
{
return false;
}
}
//---------------------------------------------------
void reset_istream()
{
if(cin.eof())
{
cin.clear();
}
else
{
cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), '\n');
}
}
//---------------------------------------------------
void wait_for_enter()
{
cout << "press <enter> to continue...";
// Reset failstate, just in case.
cin.clear();
string line;
getline( cin, line);
}
If you don't know what
const and
& is then nevermind. You can remove these two things. Program will still work. Basically I am optimizing transfer speed (and memory usage!) and I am preventing my passed values to be modified.
substr(start, charcount) does this:
test = myString.substr(0, 1); //copy ONE character starting at location 0 to test.
test = myString.substr(5, 3); //copy THREE characters starting at location 5 to test.
Remember, arrays start with index 0 and end with size()-1!!!
a word on using do/while/for loops
In this reverse() function, and yours and many others, we used a while loop. I don't like it in this case. Why?
Remember I said programming is communicating?
Well, in this very while-loop, we guarantee the loop to finish if an index (len in our case) has reached a certain value. That value is
guaranteed to be decreased to 0, if the machine doesn't break down or something. And it is also guaranteed that the Len value is decreased by 1, on every iteration. So since we have a guaranteed loop with a FIXED number of iterations, we should make it explicit by using a for-loop.
Why do you bother Val?
Well, a while-loop tells me:
"Hey, a while-loop, so it exits on a special condition. I do NOT know how many iterations are needed for this while-loop in advance, before this condition is met."
But that's wrong. We DO know when it ends. It ends when Len==0. And there is no special condition: Len is decreased by 1 on every iteration. This all makes me believe that the while-loop should be made explicit: use a for-loop.
your final job
Modify my reverse_stdSTring() function so it works with a for-loop instead of a while-loop.
Good luck!
Linear Mode
