How to improve this code

[mandir_wahin_banayenge]

Any suggestions welcome. Sorry if google groups screws up the
formatting.
TIA,
MWB

#include <iostream>
#include <list>

using namespace std;

void
print_perm(const list<int> &p)
{
list<int>::const_iterator t;
for(t=p.begin(); t != p.end() ; t++)
cout<< *t << ' ';
cout<<endl;
}

void
get_perm(int i,int n,list<int> &p)
{
list<int>::iterator t=p.begin();
if(i>n) return;
for(int k=0;k<i;k++)
{
p.insert(t++,i);
if (i==n) print_perm(p);
else get_perm(i+1,n,p);
p.remove(i);
}
}


void
gen_perm(int n)
{
list<int> p;
get_perm(1,n,p);
}

int
main(int argc,char *argv[])
{

int n=5;
gen_perm(n);
return 0;
}

 

[Victor Bazarov]

Any suggestions welcome. Sorry if google groups screws up the
formatting.

Formatting is fine. The code doesn't work. You need to fix
it first. Then you can think of improving it...

TIA,
MWB

#include <iostream>
#include <list>

using namespace std;

void
print_perm(const list<int> &p)
{
list<int>::const_iterator t;
for(t=p.begin(); t != p.end() ; t++)
cout<< *t << ' ';
cout<<endl;
}

void
get_perm(int i,int n,list<int> &p)
{
list<int>::iterator t=p.begin();
if(i>n) return;
for(int k=0;k<i;k++)
{
p.insert(t++,i);

If the list 'p' is empty (and that's the case when this function
is called for the first time), 't' evaluates to 'p.end()'. That
iterator is _not_ incrementable.

if (i==n) print_perm(p);
else get_perm(i+1,n,p);
p.remove(i);
}
}


void
gen_perm(int n)
{
list<int> p;
get_perm(1,n,p);
}

int
main(int argc,char *argv[])

You don't use those arguments, why declare them?

{

int n=5;
gen_perm(n);
return 0;
}

V

 

[Kai-Uwe Bux]

Any suggestions welcome. [snip]

[code generating all permutations of (1,2,3,4,5) snipped]

You may want to have a look at the algorithms

std::next_permutation

and

std:rev_permutation


Best

Kai-Uwe Bux

 

[mandir_wahin_banayenge]

Formatting is fine. The code doesn't work. You need to fix
it first. Then you can think of improving it...

Why didn't you actually bother to run the code? You clearly didn't
understand how the code works. I don't mind being criticized, but
please use your head and if that is too difficult for you compile and
run the code before making an idiot of yourself in a public forum.

It works fine for me
Using cygwin

$ g++ -ansi -pedantic -Wall perm.cpp -o perm
$ ./perm.exe > out.txt
$ cat out.txt
5 4 3 2 1
4 5 3 2 1
4 3 5 2 1
4 3 2 5 1
4 3 2 1 5
5 3 4 2 1
3 5 4 2 1
3 4 5 2 1
3 4 2 5 1
3 4 2 1 5
5 3 2 4 1
3 5 2 4 1
3 2 5 4 1
3 2 4 5 1
3 2 4 1 5
5 3 2 1 4
3 5 2 1 4
3 2 5 1 4
3 2 1 5 4
3 2 1 4 5
5 4 2 3 1
4 5 2 3 1
4 2 5 3 1
4 2 3 5 1
4 2 3 1 5
5 2 4 3 1
2 5 4 3 1
2 4 5 3 1
2 4 3 5 1
2 4 3 1 5
5 2 3 4 1
2 5 3 4 1
2 3 5 4 1
2 3 4 5 1
2 3 4 1 5
5 2 3 1 4
2 5 3 1 4
2 3 5 1 4
2 3 1 5 4
2 3 1 4 5
5 4 2 1 3
4 5 2 1 3
4 2 5 1 3
4 2 1 5 3
4 2 1 3 5
5 2 4 1 3
2 5 4 1 3
2 4 5 1 3
2 4 1 5 3
2 4 1 3 5
5 2 1 4 3
2 5 1 4 3
2 1 5 4 3
2 1 4 5 3
2 1 4 3 5
5 2 1 3 4
2 5 1 3 4
2 1 5 3 4
2 1 3 5 4
2 1 3 4 5
5 4 3 1 2
4 5 3 1 2
4 3 5 1 2
4 3 1 5 2
4 3 1 2 5
5 3 4 1 2
3 5 4 1 2
3 4 5 1 2
3 4 1 5 2
3 4 1 2 5
5 3 1 4 2
3 5 1 4 2
3 1 5 4 2
3 1 4 5 2
3 1 4 2 5
5 3 1 2 4
3 5 1 2 4
3 1 5 2 4
3 1 2 5 4
3 1 2 4 5
5 4 1 3 2
4 5 1 3 2
4 1 5 3 2
4 1 3 5 2
4 1 3 2 5
5 1 4 3 2
1 5 4 3 2
1 4 5 3 2
1 4 3 5 2
1 4 3 2 5
5 1 3 4 2
1 5 3 4 2
1 3 5 4 2
1 3 4 5 2
1 3 4 2 5
5 1 3 2 4
1 5 3 2 4
1 3 5 2 4
1 3 2 5 4
1 3 2 4 5
5 4 1 2 3
4 5 1 2 3
4 1 5 2 3
4 1 2 5 3
4 1 2 3 5
5 1 4 2 3
1 5 4 2 3
1 4 5 2 3
1 4 2 5 3
1 4 2 3 5
5 1 2 4 3
1 5 2 4 3
1 2 5 4 3
1 2 4 5 3
1 2 4 3 5
5 1 2 3 4
1 5 2 3 4
1 2 5 3 4
1 2 3 5 4
1 2 3 4 5

 

[Gavin Deane]

Why didn't you actually bother to run the code? You clearly didn't
understand how the code works. I don't mind being criticized, but
please use your head and if that is too difficult for you compile and
run the code before making an idiot of yourself in a public forum.

Whether he compiled the code or not, Victor is correct. The code in
question is

list<int>::iterator t=p.begin();
if(i>n) return;
for(int k=0;k<i;k++)
{
p.insert(t++,i);

The first time this is executed, the list p is empty and so the
iterator t is initialised to p.end() (which is the same thing as
p.begin() for an empty container). Incrementing p.end(), as you do on
the last line I have quoted, is undefined behaviour. Anything can
happen, including exactly what you expect. I compiled and ran your code
and the undefined behaviour manifested itself in a different way. The
program crashed on that line.

It works fine for me

Purely because you were unlucky.

Gavin Deane

 

[Victor Bazarov]

Why didn't you actually bother to run the code?

I did. It didn't work.

You clearly didn't
understand how the code works.

It does NOT.

I don't mind being criticized, but

Oh, *really*?! All indications to the contrary.

please use your head

You mean, like you use yours?

and if that is too difficult for you compile and
run the code before making an idiot of yourself in a public forum.
ROFLMAO

It works fine for me
[..]

<wiping tears of my eyes> I am sorry for you then. Come back when
you learn some manners.

V

 

[DirtyClamDigger]

Agreed - the empty list causes a 'nonincrementable iterator' debug
assertion when first compiled and attempted to run. Easy enough fix for
the code - not so sure about fixing the person's manners tho. Clearly
have never heard (or understood) the expression about gift horses &
mouths....