Member Avatar for Johnathon332

Hi guys, I was wondering if this was a good place to use a goto statement? I know this will get stuck in an infinite loop eventually and I am working to fix that right now. However I wanted everyones views on whether or not this is acceptable, the goto statement I meant. If not, how else would you have done it? Also is there anyway to make this code more efficient?

#include <iostream>
#include <string>
#include <time.h>
#include <vector>
#include <algorithm>

using namespace std;

vector<string> words;

void print_permutations(string s)
{
	short x=0;
	short y=0;
	vector<string>::iterator result;
	srand(time(NULL));

	no_word_found: for(short i = 0; i < s.length(); ++i){
						x = rand()%s.length(); //produces a random number between 0 and the length of the string.
						y = rand()%s.length();

						if(x != y){ //if both i and j are the same number and you xor them you lose chars.
							s[x] = s[x] ^ s[y]; //takes random characters from the string and XOR's them
							s[y] = s[y] ^ s[x]; //in order to swap characters in place without allocating memory.
							s[x] = s[x] ^ s[y];
						}
					 }

	result = find(words.begin(), words.end(), s);

	if(result == words.end()){//if string not in vector array execute code
			words.push_back(s);	
			cout << "Permutation: " << s << endl;
		}
		else if(result != words.end()) //if the permutation is in the array it will display this message 
			goto no_word_found;
}

int main()
{
	string word;
	do{
		cout << "\nPlease enter a word" << endl << "Original Word: ";
		cin >> word;
		print_permutations(word);
	  }while(word != "1");
	return 0;
}

Thank you

Edited by Johnathon332 because: n/a
  • 5 Contributors
  • 4 Replies
  • 209 Views
  • 13 Hours Discussion Span
  • Latest Post Latest Post by mike_2000_17

Dani AI

Generated

Goto is unnecessary here and is what makes the routine fragile rather than helpful. The original routine’s main issues go beyond the label: reseeding the RNG inside the function, using XOR-swaps for readability’s sake, generating biased/random permutations incorrectly, doing O(N) lookups in a growing vector, and no safe termination condition (which is what leads to the infinite loop). was right that structured loops are the right tool; was also on track suggesting deterministic generation rather than blind random retries.

Practical fixes and safer building blocks:

  • Seed the generator once (not inside the permutation function) and prefer C++11+ random utilities over C rand(). Example shuffle pattern:
#include <random>
#include <algorithm>

std::random_device rd;
std::mt19937 gen(rd());
std::shuffle(s.begin(), s.end(), gen);
  • Replace XOR-swap with std::swap or use std::shuffle / Fisher–Yates for a uniform random permutation.
  • Replace linear find in vector<string> with std::unordered_set<string> for average O(1) membership checks if deduplication is required.
  • Avoid endless retries: either stop when the number of unique permutations equals n! (compute factorial for small n) or limit attempts and bail out with a clear message.

Algorithm-level choices:

  • If the goal is to visit every permutation exactly once, generate them deterministically (lexicographic order via next_permutation or a backtracking generator) and print them without storing them all in memory. Storing all permutations is only feasible for small n (n! grows extremely quickly).
  • If a random sample without replacement is required, consider sampling indexes without replacement and unranking to permutations, or maintain a set of seen results and stop when exhausted or after a sensible attempt limit.

About setjmp/longjmp: ’s suggestion is technically C-compatible, but in C++ these bypass stack unwinding and destructors and are generally unsafe. ’s statement that they’re “not supported” is incorrect—available via the C headers—but their use in modern C++ should be avoided except for very low-level interoperability code.

Checklist: seed once, use std::shuffle/Fisher–Yates or deterministic next_permutation, avoid XOR swaps, use an unordered_set for deduplication, and add a clear termination condition instead of a goto loop.

Recommended Answers

All 4 Replies

Member Avatar for thines01

No, because it is not necessary.
You can fix that with a while statement.

Edited by thines01 because: clarity
Member Avatar for gerard4143

Because goto can have devastating results on your program and program flow its frowned on. If you really need to jump all over your program try a safe function like longjmp().

void longjmp (jmp_buf env, int val);
int setjmp ( jmp_buf env );
Edited by gerard4143 because: n/a
Member Avatar for Ancient Dragon

If you really need to jump all over your program try a safe function like longjmp().

void longjmp (jmp_buf env, int val);
int setjmp ( jmp_buf env );

Wrong. longjmp() and setjmp() are not supported in c++ program. I have used goto on very very rare occasion when within a deeply nested loops of if statements and other kinds of loops are not possible. In my 30 years programming I've used it maybe 2 or 3 times.

Edited by Ancient Dragon because: n/a
Member Avatar for mike_2000_17

No, don't use goto for such a simple purpose. If nothing more, it's better to replace it with while(true) { }; loop (and a break; statement somewhere in the loop). At the least, this makes it clear that the code is a loop, and respect indentation rules, that alone is already a good argument to motivate using a traditional loop statement (for, while, do-while) instead of a label/goto pair.

Your code does a generation of permutations of letters of a word, may I suggest you take a look at std::next_permutation, and either get inspired from it or use it directly (hints: the number of permutation is fixed (implying a fixed-length loop); and you shouldn't need a random-number generator anywhere, the sequence of all permutations is something that is easily obtained systematically and is deterministic).

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.