Can't figure out how to swap

With this program, I'm trying to open a file that contains a list of people's names, birth dates, grade numbers, and SSN. I can get it to read the pieces from the file and print them but I'm trying to get it in alphabetical order with a selection sort and it will not swap them. :/. plz help. and btw, theres certain parts in here that won't make sense why i have them in there, but that's because theres much more to the program to be done. thank you for your time

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>

#define MAX 20
#define SIZE 5

//date structure
typedef struct
{
  char month[MAX];
  int date;
  int year;
} DATE;

//student structure
typedef struct
{
  char fName[MAX];
  char lName[MAX];
  int gLevel;
  char sNumber[MAX];
  DATE bDate;
} STUDENT;

//functions
void fill(STUDENT identity[], FILE* fpIn, FILE* fpOut);
void printArr(STUDENT identity[]);
bool openFiles(FILE **pFPIn, FILE **pFPOut);
void prt(STUDENT identity);
void sort(STUDENT identity[]);
void swap(STUDENT *pN1, STUDENT *pN2);

//main function
int main(int argc, char* argv[])
{
  STUDENT identity[SIZE];
 
  FILE *fPIn, *fPOut;
  int i;
  if (openFiles(&fPIn, &fPOut))
      {
        fill(identity, fPIn, fPOut);
        printf("\nCreated by _______");
        printf("\n\n%26s %10s %15s %17s", "Name", "Level",
                "SSN", "Birthdate");
        printf("\n%26s %10s %15s %17s", "----", "-----",
                "---", "---------");
        printArr(identity);    //prints the unsorted array
        sort(identity);        //sorts the array
        printArr(identity);    //prints the sorted array

      }
 
     
  fclose(fPIn);
  fclose(fPOut);
 
  return 0;
}

//generate function
void fill(STUDENT identity[], FILE* fPIn, FILE* fPOut)
{
  int i;
 
  //for loop
  for (i = 0; i < SIZE; i++)
      {
        fscanf(fPIn, "%s%s%d%s%s%d%d",
            identity[i].fName, identity[i].lName,
            &identity[i].gLevel, identity[i].sNumber,
            &identity[i].bDate.month, &identity[i].bDate.date,
            &identity[i].bDate.year);
      }
}

//print function into array of structures
void printArr(STUDENT identity[])
{
  int i;
 
  //for loop
  for (i = 0; i < SIZE; i++)
    {
      printf("\n");
      prt(identity[i]);
   
    }
  printf("\n\n");
}

//opens the files
bool openFiles(FILE **pFPIn, FILE **pFPOut)
{
  bool success = false; 

  if ((*pFPIn = fopen("t1", "r")) != NULL)
  {
      if ((*pFPOut = fopen("t1.out", "w")) != NULL)
        success = true;
      else
        printf("\nUnable to open \"t1.out\" for writing.\n\n");
  }
  else
      printf("\nUnable to open \"t1.txt\" for reading.\n\n");
 
  return success;
}
//prints the function 
void prt(STUDENT identity)
{
 
    printf("%17s %8s %10d %15s %9s %2d %d%", identity.fName,
        identity.lName,
        identity.gLevel,
        identity.sNumber,
        identity.bDate.month,
        identity.bDate.date,
        identity.bDate.year);
}

//selection sort function
void sort(STUDENT identity[])
{

  int pass, min, trav;
 
  for (pass = 0; pass < SIZE - 1; pass++)
    {
      for (min = pass, trav = pass + 1; trav < SIZE; trav++)
        {
            if (identity[trav].lName < identity[min].lName)
              {
                  min = trav;
              }
            trav++;
                 
        }
      swap(&identity[pass], &identity[min]);
      pass++;
    }
}

//swap function
void swap(STUDENT *pN1, STUDENT *pN2)
{
  char temp;
 
  temp = pN1;
  pN1 = pN2;
  pN2 = temp;
}

---

The current swap function is the only way that I have done that would compile, it gave me warning messages however.

Just one post on the subject would be enough, I think.

--
Mats

this is what i'm getting it to print

Code:

---

$ ./a.out

Created by

                      Name      Level            SSN        Birthdate
                      ----      -----            ---        ---------
            Peter  Griffin          1    111-11-1111      May  9 1965
            Lois  Griffin          3    234-33-2345    April  2 1966
            Glenn Quagmire          7    555-41-2143  January 10 1965
              Tom  Tucker          2    999-99-9999      July  4 1970
            Adam    West        10    444-22-3434  November  4 1960


            Peter  Griffin          1    111-11-1111      May  9 1965
            Lois  Griffin          3    234-33-2345    April  2 1966
            Glenn Quagmire          7    555-41-2143  January 10 1965
              Tom  Tucker          2    999-99-9999      July  4 1970
            Adam    West        10    444-22-3434  November  4 1960

---

oops, that first one i didnt mean to send. i had clicked on preview and went back to edit i thought. its early. sorry :/

Well, I can see that the code isn't going to work, I'm just wondering if you have enabled ANY compiler warnings - as your posted code assigns a pointer to a char, then assigns two pointers, and then assigns the original char into another pointer. And most compilers should definitely warn about this code.

Think about what you are swapping, and what your temporary object is supposed to be. Also consider what the rules are for changing things outside of a function from inside the function.

--
Mats

and why to post such a long code to ask a question about function of 3 lines long?

well after a few replies on different threads by many different people. i've noticed that a lot ask for more code posted, so i thought id just cancel out that, but i apologize if it bothers you :/

and i've gotten it to give no warnings now, but it is still not swapping. baby steps :D

[CODE]
void swap(STUDENT *pN1, STUDENT *pN2)
{
STUDENT *temp[MAX];

*temp = pN1;
pN1 = pN2;
pN2 = *temp;
}
/CODE]

I doubt that compiles either (at least not without warnings). You may want to enable [and fix] warnings when you compile your code.

--
Mats

Quote:

---

Originally Posted by deathrattle

well after a few replies on different threads by many different people. i've noticed that a lot ask for more code posted, so i thought id just cancel out that, but i apologize if it bothers you :/

---

yes, we can ask to post more code, when posted code seems to be ok and problem should be in the calling function.

in your case - the problem is clearly inside your swap function, so posting just:

Code:

---

void swap(STUDENT *pN1, STUDENT *pN2)
{
  char temp;
 
  temp = pN1;
  pN1 = pN2;
  pN2 = temp;
}

---

would make it easier for anyone ready to help to see th eproblem and thus answers would be delivered faster...

I haven't even saw your function till I scrolled the code block down

as was said - your problem is type of the temp var and assigning pointers instead of oblects

Quote:

---

Originally Posted by deathrattle

and i've gotten it to give no warnings now, but it is still not swapping. baby steps :D

Code:

---

void swap(STUDENT *pN1, STUDENT *pN2)
{
  STUDENT *temp[MAX]; 
       
  *temp = pN1;
  pN1 = pN2;
  pN2 = *temp;
}

---

---

you need 1 temp variable of type STUDENT instead of array of pointers
and you should swap objects instead of pointers

Alternatively - if you really want to swap only pointers

your temp should be just one pointer - not array
and your arguments should be passed as pointer to pointer instead of just pointer
PS. but this is NOT your case... I have checked your sort function to be sure what are your real intentions... ;)

alright, i changed it from an array to just one 1 temp variable. i then changed them from pointers to just objects. i then had to change the & symbols from the calling function. it compiles with no warnings but still no swapping. and i also realized that by last name, the names i had in the file were already in order except for the two griffins. lawlz. so i switched adam wests place to the top to make it simple.

Quote:

---

Originally Posted by deathrattle

alright, i changed it from an array to just one 1 temp variable. i then changed them from pointers to just objects. i then had to change the & symbols from the calling function. it compiles with no warnings but still no swapping. and i also realized that by last name, the names i had in the file were already in order except for the two griffins. lawlz. so i switched adam wests place to the top to make it simple.

---

no - you still need to pass pointers to objects to swap them - so your function prototype was ok - do not change it, work only on the function body

alrighty, its back to pointers. again it compiles but still no swaps.

Code:

---

void swap(STUDENT *pN1, STUDENT *pN2)
{
  STUDENT temp;
         
  temp = *pN1;
  *pN1 = *pN2;
  *pN2 = temp;
}

---

Quote:

---

Originally Posted by deathrattle

alrighty, its back to pointers. again it compiles but still no swaps.

Code:

---

void swap(STUDENT *pN1, STUDENT *pN2)
{
  STUDENT temp;
         
  temp = *pN1;
  *pN1 = *pN2;
  *pN2 = temp;
}

---

---

That does look fine to me. Are you sure that you are not having some other problem compiling the code, and that is making your OLD code run? [You wouldn't be the first, I've done it more than once - try changing the code in some OBVIOUS way (e.g. add an extra printout somewhere, and confirm that this happens)].

--
Mats

i put a "WHOABUDDY" at the bottom of the swap function. it printed the list of names, WHOABUDDY twice, and then the list of names again still unsorted. the two whoabuddy's look odd to me as it is only called once. any thoughts?

Twice does seem a bit low, yes. I'd expect as many swaps as you have iterations, which should be 5.

Code:

---

  for (pass = 0; pass < SIZE - 1; pass++)
    {
      for (min = pass, trav = pass + 1; trav < SIZE; trav++)
        {
            if (identity[trav].lName < identity[min].lName)
              {
                  min = trav;
              }
            trav++;
                 
        }
      swap(&identity[pass], &identity[min]);
      pass++;
    }

---

Do you actually want the red bit?

--
Mats

allllright, now we're gettin somewhere. i took out the pass++; and more whoabuddys came up but it still wasnt swapping. in the call of the swap function, i changed &identity[pass] to &identity[pass + 1] and it swapped only one of the names out of order, whichever one is at the top to its correct place.

Quote:

---

Originally Posted by deathrattle

allllright, now we're gettin somewhere. i took out the pass++; and more whoabuddys came up but it still wasnt swapping. in the call of the swap function, i changed &identity[pass] to &identity[pass + 1] and it swapped only one of the names out of order, whichever one is at the top to its correct place.

---

Then perhaps you should fix the other loop too - which has the same problem.

--
Mats

are you referring to the trav++; after the if? i'm playing around with all kinds of stuff and cannot figure out which section of the loop you are referring to.

Yes, you have two trav++ in your for-loop, one in the for-statement itself, and one just before the end brace. Whilst this is within the C language specification, I'm 99.9% sure that you do not want to do that.

--
Mats

hen you use a for loop, you don't have to iterate the variable inside the loop, the setup of the loop does that for you

Code:

---

for (var = 0; var < MAX; var++) { // Note the var++ here!
  // Do Something
  // No var++; needed!!!
}

---

If you used a while loop then you'd need to increment tha variable inside the loop...

Code:

---

var = 0;
while (var < MAX) {
  // Do Something
  var++;
}

---

These two loops are exactly the same functionally.

Quote:

---

Originally Posted by Bladactania

hen you use a for loop, you don't have to iterate the variable inside the loop, the setup of the loop does that for you

Code:

---

for (var = 0; var < MAX; var++) { // Note the var++ here!
  // Do Something
  // No var++; needed!!!
}

---

If you used a while loop then you'd need to increment tha variable inside the loop...

Code:

---

var = 0;
while (var < MAX) {
  // Do Something
  var++;
}

---

These two loops are exactly the same functionally.

---

I agree - almost every time a for-loop-variable is changed outside of the for-statement, it is in error.

--
Mats

Generally for loops are used when you want to perform the same action a set number of times (like you do in your code). Generally while loops are used when you want to continue to perform a given action until a certain result is obtained. It is a fine, yet clear, distinction.

i removed the second trav++; and it is still only swapping the top name that is out of order to its correct spot.

This is what your sort function should look like:

Code:

---

  int pass, trav;
 
  for (pass = 0; pass < SIZE - 1; pass++)
  {
      for (trav = pass + 1; trav < SIZE; trav++)
      {
        if (identity[pass].lName > identity[trav].lName)
        {
            swap(&identity[pass], &identity[trav]);                   
        }
      } //end of inner for
  }  //end of outer for

---

You never need to increment pass, trav, or anything else, outside the for loops.
Min is not needed at all.

This is my favorite flavor of bubblesort. Please don't muck around with it. :)

If you have a lot of empty array slots, they will be the first things you see after the sort, but that doesn't mean the sort is not working.

i tried your code and it didn't swap any at all. im trying to do it with a selection sort instead of bubble if that is at all possible.

Quote:

---

Originally Posted by deathrattle

i tried your code and it didn't swap any at all. im trying to do it with a selection sort instead of bubble if that is at all possible.

---

Post your code up. I've used that particular version of sorting for almost three decades :)

It's selection sort, (sort of), but it sweeps all the way through the array on the inner loop, so it falls into the performance of the bubblesort "bunch".

If you want selection sort, that's fine.

You know you have to compare strings in C with strcmp(), right? You can't just compare name to name, directly.