SortByName function...

M

Maria Mela

Hello everybody...

What´s wrong in may sort function??
I call this function, and after i list all stuct... but "sortbyname"
function doesn´t do nothing... why??
can you help me??

Thks
Kisses

void sortbyname (void)
{
int sort_Counter = 0, i = 0;
if(firsta==NULL)
{
puts("Nao existem Registos!");
return;
}
currenta = temp;
while(currenta != NULL)
{
sort_Counter++;
currenta = currenta->next;
}
currenta = temp;
for(;i < sort_Counter;i++)
{
while(currenta != NULL)
{
if(currenta->next != NULL)
{
if( strcmp(currenta->nome, currenta->next->nome) > 0 )
{
temp = (struct aluno *) malloc( sizeof(struct aluno ) );
if( temp != NULL )
{
strcpy( temp->nome,currenta->nome );
strcpy( temp->curso, currenta->curso );
strcpy( currenta->nome, currenta->next->nome );
strcpy( currenta->curso, currenta->next->curso );
}
}
}
currenta = currenta->next;
}
currenta = temp;
}
}
 
G

gw7rib

Hello everybody...

What´s wrong in may sort function??
I call this function, and after i list all stuct... but "sortbyname"
function doesn´t do nothing... why??
can you help me??

Thks
Kisses

I haven't looked at the code in great detail, but there are a few
things I noticed that may point you on your way.
void sortbyname (void)
{
int sort_Counter = 0, i = 0;
if(firsta==NULL)
{
puts("Nao existem Registos!");
return;
}
currenta = temp;

Are you sure "temp" is what you mean here? By the look of it you would
be better off with "firsta", which I am assuming is the pointer to the
first item in your list.
while(currenta != NULL)
{
sort_Counter++;
currenta = currenta->next;
}
currenta = temp;

Same comment here.
for(;i < sort_Counter;i++)
{
while(currenta != NULL)
{
if(currenta->next != NULL)
{
if( strcmp(currenta->nome, currenta->next->nome) > 0 )
{
temp = (struct aluno *) malloc( sizeof(struct aluno ) );
if( temp != NULL )
{
strcpy( temp->nome,currenta->nome );
strcpy( temp->curso, currenta->curso );
strcpy( currenta->nome, currenta->next->nome );
strcpy( currenta->curso, currenta->next->curso );

Firstly - this all seems a bit unnecessary. What you are doing here is
leaving the items where they are and swapping the text over. The
normal technique with a linked list is to move the pointers around so
the items themselves don't have to move.

For example, suppose A points to B, B points to C, and C points to D.
You decide that B and C are the wrong way round. So you change it so
that A points to C, C points to B, and B points to D. There is no need
to change the text of any of the items, or to move them around, you
just need to change what A, B and C point to.

If you really do want to swap the text over, I think you have missed
out two lines. You need, after the lines above, to copy the text from
temp to currenta -> next.
}
}
}
currenta = currenta->next;
}
currenta = temp;
}



}

Hope that helps.
Paul.
 
J

Joerg Schoen

Maria said:
void sortbyname (void)
{
int sort_Counter = 0, i = 0;
if(firsta==NULL)
{
puts("Nao existem Registos!");
return;
}
currenta = temp;
while(currenta != NULL)
{
sort_Counter++;
currenta = currenta->next;
}
currenta = temp;
for(;i < sort_Counter;i++)
{
while(currenta != NULL)
{
if(currenta->next != NULL)
{
if( strcmp(currenta->nome, currenta->next->nome) > 0 )
{
temp = (struct aluno *) malloc( sizeof(struct aluno ) );
if( temp != NULL )
{
strcpy( temp->nome,currenta->nome );
strcpy( temp->curso, currenta->curso );
strcpy( currenta->nome, currenta->next->nome );
strcpy( currenta->curso, currenta->next->curso );

Here you do not copy temp back to currenta->next which seems to be the
main problem.
}
}
}
currenta = currenta->next;
}
currenta = temp;
}
}

General observation is that you do not use local variable properly,
instead you have globals all over the place. This is rather bad style.

Also, you have memory leaks here: You never free "temp" in your routine.
There is no need to allocate it every time. Just use a declaration like
struct aluno temp;
and later write
strcpy(temp.nome, currenta->nome);
etc.

Finally, you have programmed a bubble sort for linked lists. Bad idea.
For linked lists, mergesort is the right choice. I opened a thread two
weeks ago in comp.lang.c about an efficient mergesort of linked lists.
Maybe you have a look. Some people put some nice ideas there.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,432
Messages
2,571,680
Members
48,796
Latest member
Greg L.

Latest Threads

Top