6
\$\begingroup\$

I am using this code to display all the permutations of a given input string. Any advice/solution will be very helpful.

#include<iostream>
using namespace std;

/* Function to swap two characters */
void swap(char& a, char& b)
{
 char temp;
temp = a;
a = b;
b = temp;
}    
/* Function to obtain permutations of string characters */
void permutation(string s,int i,int n)
{
int j;
if (i == n)
    cout << s << "\t";
else
  {
    for (j = i; j < s.length(); j++)
    {
        swap(s[i],s[j]);
        permutation(s, i + 1, n);
        swap(s[i],s[j]);
    } 
  }
}  
int main()
{
 string s;
 cout << "Enter the amino acid string : ";
 cin >> s;
 cout << endl << "The permutations of the given string : " << endl;
 permutation(s, 0, s.length() - 1);
 cout << endl;
}
\$\endgroup\$
4
\$\begingroup\$

Note that using the standard library function std::next_permutation from the header <algorithm>, your code can more or less be reduced to:

#include <algorithm>
#include <iostream>
#include <string>

int main()
{
    std::string s;
    std::cout << "Enter the amino acid string: ";
    std::cin >> s;
    std::cout << "\nThe permutations of the given string:\n";

    std::sort(s.begin(), s.end());
    do
    {
        std::cout << s << '\n';
    } while(std::next_permutation(s.begin(), s.end()));
}

Note that the iterable (the string in our case) needs to be sorted for the call to std::next_permutation to work, hence the call to std::sort beforehand.


That said, solving your problem in a more idiomatic manner might be interesting, but there are some more things you may want to know if you want to write idiomatic C++ in the future:

  • You don't need to write your own swap function. The standard already provides std::swap to swap any two values.

  • using namespace std; is often considered bad practice. While it shouldn't matter in your case, using it in a header file will drag every name from the std:: namespace into the global namespace in every file including this header file. Generally speaking, unless you know what you're doing, avoid this construct and fully qualify every name from the standard library with std::.

  • You could use some additional spacing. Don't hesitate to separate your functions by empty lines and to add some more spaces between your functions parameters. It will make your code easier to read. Also, try to be consistent when you indent stuff; use the same number of spaces for indentation everywhere.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ To clarify/expand a little: next_permutation will work fine at permuting the string if the input isn't sorted. The only part that won't work is detecting when you've iterated through all possible permutations (i.e., next_permutation returns false when transitioning to sorted order). \$\endgroup\$ – Jerry Coffin Aug 21 '15 at 1:32
0
\$\begingroup\$

Warning: By declaring using namespace std, you are also importing std::swap. This can cause errors, because your function is also called swap. Use std:: before the things you need the standard namespace for, and remove using namespace std.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.