Letter swapper code not working. Help!

[SirEnder125]

Hello. I made this code:

/*
code by Alexander123
*/

#include <include.h> // custom header (includes iostream, etc.)

void Test(string& word)
{
	for(int i=0; i<=word.size(); i++)
	{
		char temp = word[0];
		word[i] = word[++i];
		i--;
		if(i == word.size())
		{
			word[i] = temp;
		}
		cout << word;
		Sleep(100);
		cls(); // clears screen, declared in include.h
	}
}

int main()
{
	string s = "Hello world!";
	Test(s);
}

Run this code by replacing <include.h> with iostream and string.
Use namespace std.

My problem is a runtime-error, instead of moving a message across the
screen (changing the order of the letters), it moves the letters each back one space and then stops. Can anyone spot the problem???


Thanks.

EDIT: Running the code by changing include.h to iostream, etc. causes a compile-time error (At least in cpp.sh it does).

[jonnin]

what is it you want to do?

does include.h pull in namespace std? This is likely your problem, if it does... you need to put std:: on the items you used, or pull in the part of the namespace you need eg using std::cout
also if include defines things like cls, you need it. That is not a standard c++ thing. I don't get what you want to do with the include part.

your loop is like a pop quiz on convoluted code to see if students can follow it.
changing the loop variable in the loop all over the place in random ways makes it easy to inject a bug and difficult to find the bug.

remember too that [size] isnt correct. c++ indexes from zero. if a word has 5 letters, they are [0] through [4] (0,1,2,3,4) IS 5 THINGS. line 14 is probably wrong.

it looks like you have a bunch of code to do this, but again, I am half guessing due to the code being funky:

string  s = "hello world";
   char tmp = s[0];
   s = s.substr(1,s.size()-1);
   s+= tmp;
   cout << s << endl;
   

is ^^ that what you wanted to do?

[dutch]

#include <iostream>
#include <string>
#include <thread>
#include <chrono>
using namespace std;

void marquee(string& word)
{
    using namespace std::chrono_literals;
    for (size_t start = 0; start < word.size(); ++start)
    {
        for (size_t i = 0; i < word.size(); ++i)
            cout << word[(start + i) % word.size()];
        cout << flush;
        this_thread::sleep_for(200ms);
        for (size_t i = 0; i < word.size(); ++i)
            cout << '\b';
    }
}

int main()
{
    string s = "Hello world! ";
    for (int i = 0; i < 10; ++i)
        marquee(s);
}

[SirEnder125]

What I want to do is this:

"Hello world! "
(clear console)
"ello world! H"
(clear console)
"llo world! He"
(clear console)
"lo world! Hel"

etc.

And this code:

/*
code by Alexander123
*/

#include <include.h> // custom header (includes iostream, etc.)

void Mix(string& s) // function with jonnin's code
{
	for(int i=0; i<100; i++)
	{
		char tmp = s[0];
   		s = s.substr(1,s.size()-1);
   		s+= tmp;
   		cout << s << std::endl;
   		Sleep(100);
   		cls();
	}
}

int main()
{
	string s = "Hello world!";
	Mix(s);
}

does exactly that.

Thanks!

[seeplus]

Sleep() needs Windows - why not use C++ sleep_for?

Also, this code using substr which is very inefficient. If all you want is to display the rotated string then just display the required chars. Eg (based upon dutch's code)

#include <iostream>
#include <string_view>
#include <thread>
#include <chrono>

void marquee(std::string_view word)
{
	using namespace std::chrono_literals;

	for (size_t start = 0; start < word.size(); ++start)
	{
		for (size_t i = 0; i < word.size(); ++i)
			std::cout << word[(start + i) % word.size()];

		std::cout << std::endl;
		std::this_thread::sleep_for(200ms);
	}
}

int main()
{
	marquee("Hello world ");
}

[jonnin]

yea I didn't realize you wanted to loop it when I put that in, dutch's code is better if you want to spam the rotation.
or if you really want to go speed freak on it (which apparently you do not given the sleeps) you can make an array of the rotated strings one time and then iterate them. ye olde time space tradeoff style.

given the sleep, even the substr inefficiency may not matter, but its good to keep efficiency in mind even if not necessary.

[SirEnder125]

Thanks.