-1

I want to model a simple classroom line with my program.

  • function first: Put the student x in front of the line.

  • function out: Delete the student x from the line.

  • function backToClassroom: Print the student in the line.

  • function reverse: Reverse the students order in the line.

  • function place: Student x takes a place for the student y behind him/herself then if student y want to join the line, he/she should come behind x.

  • function add: Add student x at the end of the line, unless a student takes a place for him/herself.

My problems is:

I don't know how should I code function add and place correctly as I explain in top.


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

struct line {
    char name[100];
    struct line *nextPtr;
};

typedef struct line Line;
typedef Line *LinePtr;

void add(LinePtr *lPtr, char array[100]);
void out(LinePtr *lPtr, char array[100]);
int isEmpty(LinePtr lPtr);
void backToClassroom(LinePtr currentPtr);
void first(LinePtr* lPtr, char array[100]);
void place(LinePtr previousPtr, char array[100]);
static void reverse(LinePtr* lPtr);

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

    char order[25];
    LinePtr startPtr = NULL;
    char tempName1[100];
    char tempName2[100];

    gets(order);

    while(strcmp(order, "back to classroom") != 0) {

        if(strcmp(order, "add") == 0) {
            scanf("%s", tempName1);
            add(&startPtr, tempName1);
        }

        if(strcmp(order, "out") == 0) {
            scanf("%s", tempName1);
            out(&startPtr, tempName1);
        }

        if(strcmp(order, "first") == 0) {
            scanf("%s", tempName1);
            first(&startPtr, tempName1);
        }

        if(strcmp(order, "place") == 0) {
            scanf("%s", tempName1);
            scanf("%s", tempName2);
            place(startPtr, tempName2);
        }

        if(strcmp(order, "reverse") == 0) {
            reverse(&startPtr);
        }

        gets(order);
    }

    if(strcmp(order, "back to classroom") == 0) {
        backToClassroom(startPtr);
    }

    return 0;
}


int isEmpty(LinePtr lPtr) {
    return (lPtr == NULL);
}

void backToClassroom(LinePtr currentPtr) {
    if(isEmpty(currentPtr)) {
        printf("line is empty.\n");
    }
    else {
        while(currentPtr != NULL) {
            printf("%s\n", currentPtr->name);
            currentPtr = currentPtr->nextPtr;
        }
    }
}

static void reverse(LinePtr* lPtr) {
    LinePtr previousPtr = NULL;
    LinePtr currentPtr = *lPtr;
    LinePtr afterPtr;
    while(currentPtr != NULL) {
        afterPtr  = currentPtr->nextPtr;  
        currentPtr->nextPtr = previousPtr;   
        previousPtr = currentPtr;
        currentPtr = afterPtr;
    }
    *lPtr = previousPtr;
}

void out(LinePtr *lPtr, char array[100]) {
    LinePtr tempPtr;
    if(strcmp(array, (*lPtr)->name) == 0) {
        tempPtr = *lPtr;
        *lPtr = (*lPtr)->nextPtr;
        free(tempPtr);
    }
    else {
        LinePtr previousPtr = *lPtr;
        LinePtr currentPtr = (*lPtr)->nextPtr;
        while((currentPtr != NULL) && ((strcmp(currentPtr->name, array)) != 0)) {
            previousPtr = currentPtr;
            currentPtr = currentPtr->nextPtr;
        }
        if(currentPtr != NULL) {
            tempPtr = currentPtr;
            previousPtr->nextPtr = currentPtr->nextPtr;
            free(tempPtr);
        }
    }
}

void first(LinePtr* lPtr, char array[100]) {
    LinePtr newPtr = malloc(sizeof(Line));
    strcpy(newPtr->name, array);
    newPtr->nextPtr = (*lPtr);
    (*lPtr) = newPtr;
}

void place(LinePtr previousPtr, char array[100]) {
    if(previousPtr == NULL) { 
       printf("the given previous node cannot be NULL");
       return;      
    }  
    LinePtr newPtr = (LinePtr) malloc(sizeof(Line));
    strcpy(newPtr->name, array);
    newPtr->nextPtr = previousPtr->nextPtr; 
    previousPtr->nextPtr = newPtr;
}

void add(LinePtr *lPtr, char array[100]) {
    LinePtr newPtr = malloc(sizeof(Line));
    LinePtr lastPtr = *lPtr;
    strcpy(newPtr->name, array);
    newPtr->nextPtr = NULL;
    if(*lPtr == NULL) {
       *lPtr = newPtr;
       return;
    }  
    while(lastPtr->nextPtr != NULL) {
        lastPtr = lastPtr->nextPtr;
    }
    lastPtr->nextPtr = newPtr;    
}
john
  • 3
  • 2
  • Please edit your post to have proper capitalization and punctuation. It's hard to read in its current state. – user694733 Jan 24 '18 at 08:21
  • @user694733 Hi, Is it OK now? – john Jan 24 '18 at 08:26
  • **Never, never, never** use `gets()` it is so insecure and prone to buffer overrun it has been removed from the C11 library. Learn how to use `fgets` and trim the trailing `'\n'` it includes. It takes no more than 5-lines of code. If your professor suggested `gets()`, stop - drop the class while there is still time -- and go find a competent professor to teach you C. – David C. Rankin Jan 24 '18 at 18:46
  • @DavidC.Rankin Hi, actually I think I should do what you suggest. and I fix that and use **sscanf** instead. Thanks. – john Jan 24 '18 at 19:59
  • That works too, but remember using `scanf` for user input is full of pitfalls for the unwary. If you are reading lines (generally what a user inputs) or lines from a file, a *line-oriented* input function (`fgets` or POSIX `getline`) is the proper choice. For numeric data, `scanf` provides a nice shortcut for `fgets/sscanf`, but know it leaves the trailing `'\n'` unread in `stdin` just waiting to torpedo your next attempt to read a character with `%c` or string with `%s` unless you account for it in your *format string*. – David C. Rankin Jan 24 '18 at 20:24
  • 1
    `typedef Line *LinePtr;` You will want to review: [Is it a good idea to **typedef** pointers?](http://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers). While not an error, the standard coding style for C avoids the use of `camelCase` or `MixedCase` variable names in favor of all *lower-case* while reserving *upper-case* names for use with macros and constants. example: [**NASA - C Style Guide, 1994**](http://homepages.inf.ed.ac.uk/dts/pm/Papers/nasa-c-style.pdf) – David C. Rankin Jan 24 '18 at 20:28

3 Answers3

0

I think all you need is this:

void place(LinePtr previousPtr, char array[100]) 
{
    return add (previousPtr, array);
}

// If lPtr == null then this is an insertion at the end of the list
// If lPtr != null then this is an insertion into the list after lPtr.
void add(LinePtr *lPtr, char array[100]) 
{
    LinePtr newPtr = malloc(sizeof(Line));
    strcpy(newPtr->name, array);
    newPtr->nextPtr = NULL;

    LinePtr lastPtr = *lPtr;
    if(*lPtr == NULL) 
    {
       *lPtr = newPtr;
       return;
    }  
    while(lastPtr->nextPtr != NULL) {
        lastPtr = lastPtr->nextPtr;
    }
    lastPtr->nextPtr = newPtr;
}

Your add function already does an add or insert.

Code Gorilla
  • 962
  • 9
  • 23
0

The place function has to know the old node. You can supply a new name as well as the old name.

//place before the other name
void place(LinePtr *list, char *newName, char *otherName)
{
    if(list == NULL)
    {
        printf("the given previous node cannot be NULL");
        return;
    }

    Line *walk = *list;
    Line *prev = NULL;
    while(walk)
    {
        if(strcmp(walk->name, otherName) == 0)
        {
            //found
            LinePtr newPtr = (LinePtr)malloc(sizeof(Line));
            strcpy(newPtr->name, newName);
            newPtr->nextPtr = walk;
            if(prev)
                prev->nextPtr = newPtr;
            else
                *list = newPtr;
            break;
        }
        prev = walk;
        walk = walk->nextPtr;
    }
}

void place_after_otherName(LinePtr *list, char *newName, char *otherName)
{
    if(list == NULL)
    {
        printf("the given previous node cannot be NULL");
        return;
    }

    Line *walk = *list;
    while(walk)
    {
        if(strcmp(walk->name, otherName) == 0)
        {
            //found
            LinePtr newPtr = (LinePtr)malloc(sizeof(Line));
            strcpy(newPtr->name, newName);
            newPtr->nextPtr = walk->nextPtr;
            walk->nextPtr = newPtr;
            break;
        }
        walk = walk->nextPtr;
    }
}

The add function works as expected. Example:

int main(void) 
{
    LinePtr startPtr = NULL;
    add(&startPtr, "x");
    add(&startPtr, "y");
    add(&startPtr, "z");

    place(&startPtr, "A", "z");
    place_after_otherName(&startPtr, "B", "z");

    LinePtr p = startPtr;
    while(p)
    {
        printf(p->name);
        p = p->nextPtr;
    }

    printf("\n");
    return 0;
}
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77
  • Hi, it actually works. thanks. but there is a problem , I still need that specific condition that I explain. after user type **place jack john** _john_ shouldn't be on the line unless user type **add john** then only after that _john_ place on that specific position. could you tell me how to do that ? thanks. – john Jan 24 '18 at 19:52
0

John, your code is quite awkward, the place and out commands do not work as intended and neither out or back to classroom free any of the memory you have allocated. Get in the habit now of tracking each allocation you make and then free each before the program ends. Yes, the memory is freed when your program exits, but when you begin writing functions (as here) where you allocate memory, you must free that memory or as your code grows, memory management will rapidly become unmanageable. (so do yourself a favor, learn to do it right from the beginning)

Next, in any interface you write where you expect the user to enter input, prompt the user for the input. Otherwise, the user is left looking at a blinking cursor on the screen wondering if the program is stuck??

Thinking though what data you need and logically providing prompts to the user will help you lay your code out in a less awkward fashion.

Do not typedef pointers. (e.g. typedef Line *LinePtr;) That will only serve to confuse the heck out of you as to the actual level of pointer indirection, and it also makes your code almost impossible for someone else reading through your code. (this becomes a much bigger problem as you code is spread between 10 different header and source files) Especially while you are learning C do not typedef pointers

Let's turn to your main program loop. First, since you do not make use of int argc, char *argv[], the proper invocation of main is int main(void) making it explicit that no arguments are expected. Applying the prompting discussed above, the style changes from my comment and reordering to make a bit more sense, your main could look like:

#define ORDC  25    /* if you need constants, define them or use an enum */
#define NAMC 100    /*      (don't use magic numbers in your code!)     */
...
int main (void) {

    char order[ORDC] = "",  /* initialize all strings to zeros */
        name1[NAMC] = "",
        name2[NAMC] = "";
    line *startptr = NULL;

    /* provide an initial prompt showing valid orders to place */
    printf ("orders [add, out, first, place, reverse, back to classroom]\n");

    for (;;) {  /* loop until done or user cancels input */

        printf ("\norder: ");               /* prompt for order: each iteration */
        if (!fgets (order, sizeof order, stdin)) {  /* use fgets for user input */
            fprintf (stderr, "note: user canceled input.\n");
            return 1;
        }
        rmcrlf (order);        /* trim the trailing '\n' (returning the length) */

        if (strcmp (order, "back to classroom") == 0) {         /* are we done? */
            backtoclassroom (startptr);
            break;
        }

        if (strcmp (order, "reverse") == 0) {          /* reverse takes no name */
            reverse (&startptr);
            continue;
        }

        printf ("name : ");        /* every other function takes a student name */
        if (!fgets (name1, sizeof name1, stdin)) {
            fprintf (stderr, "note: user canceled input.\n");
            return 1;
        }
        rmcrlf (name1);

        if (strcmp (order, "add") == 0)          /* use if, else if, else logic */
            add (&startptr, name1);
        else if (strcmp (order, "out") == 0)
            out (&startptr, name1);
        else if (strcmp (order, "first") == 0)
            first (&startptr, name1);
        else if (strcmp (order, "place") == 0) {      /* place takes two names */
            printf ("after: ");
            if (!fgets (name2, sizeof name2, stdin)) {       /* get name2 here */
                fprintf (stderr, "note: user canceled input.\n");
                return 1;
            }
            rmcrlf (name2);
            place (startptr, name1, name2);
        }
        else    /* handle the Bad Input case */
            fprintf (stderr, "error: invalid order, try again.\n");
    }

    return 0;
}

When you pass an array to a function as a parameter, the first level of indirection (meaning the first [..] following the array name) is converted to a pointer (in fact for any access of an array, other than with sizeof, _Alignof or the unary & operator, or is a string literal used to initialize an array, this conversion takes place, see: C11 Standard - 6.3.2.1 Lvalues, arrays, and function designators (p3)). So your functions need only take a pointer to the array as a parameter, e.g.

void first (line **lptr, char *name);
void place (line *lptr, char *name1, char *name2);

(and use descriptive names for your variables, name instead of array makes a lot more sense)

For your out function, you only have two cases you are dealing with, (1) am I deleting the first node? (if so, the list address will become that of the 2nd node), or (2) iterate until I find the node to delete and then prev->nextptr = current->nextptr; free (current);. You could do something like:

void out (line **lptr, char *name) 
{
    line *iter = *lptr,
        *prev  = *lptr;

    if (strcmp ((*lptr)->name, name) == 0) {    /* name is first node */
        iter = iter->nextptr;                   /* save pointer to next */
        free (*lptr);                           /* free first */
        *lptr = iter;                           /* set first = saved */
        return;
    }

    while (iter && strcmp (iter->name, name)) { /* find node with name */
        prev = iter;                            /* save previousptr */
        iter = iter->nextptr;
    }
    if (!iter) {    /* handle name not found */
        fprintf (stderr, "error: %s not in list - can't remove.\n", name);
        return;
    }

    prev->nextptr = iter->nextptr;      /* previousptr = nextptr */
    free (iter);                        /* free current */
}

For your place function, you need 2 names, (1) name1, the name of the new student, and (2) name2, the name of the person to place them after. Adding the needed parameter to place, you could do something like the following:

void place (line *lptr, char *name1, char *name2) 
{
    line *iter = lptr;
    line *newptr = malloc (sizeof *newptr);
    strcpy (newptr->name, name1);
    newptr->nextptr = NULL;

    while (iter && strcmp (iter->name, name2))  /* locate after: name */
        iter = iter->nextptr;

    if (!iter) {    /* handle name2 not found */
        fprintf (stderr, "error: %s not in list - can't place %s.\n", 
                name2, name1);
        return;
    }

    newptr->nextptr = iter->nextptr;
    iter->nextptr = newptr;
}

That solves your two pressing issues. There may be a corner-case or two you will need to tweak, but putting all the pieces together (and adding spaces between your function names and the ( so older eyes can read your code better, you could do something like the following:

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

#define ORDC  25    /* if you need constants, define them or use an enum */
#define NAMC 100    /*      (don't use magic numbers in your code!)     */

typedef struct line {
    char name[NAMC];
    struct line *nextptr;
} line;

size_t rmcrlf (char *s);

void add (line **lptr, char *name);
void out (line **lptr, char *name);
int isempty (line *lptr);
void backtoclassroom (line *currentptr);
void first (line **lptr, char *name);
void place (line *lptr, char *name1, char *name2);
static void reverse (line **lptr);

int main (void) {

    char order[ORDC] = "",  /* initialize all strings to zeros */
        name1[NAMC] = "",
        name2[NAMC] = "";
    line *startptr = NULL;

    /* provide an initial prompt showing valid orders to place */
    printf ("orders [add, out, first, place, reverse, back to classroom]\n");

    for (;;) {  /* loop until done or user cancels input */

        printf ("\norder: ");               /* prompt for order: each iteration */
        if (!fgets (order, sizeof order, stdin)) {  /* use fgets for user input */
            fprintf (stderr, "note: user canceled input.\n");
            return 1;
        }
        rmcrlf (order);        /* trim the trailing '\n' (returning the length) */

        if (strcmp (order, "back to classroom") == 0) {         /* are we done? */
            backtoclassroom (startptr);
            break;
        }

        if (strcmp (order, "reverse") == 0) {          /* reverse takes no name */
            reverse (&startptr);
            continue;
        }

        printf ("name : ");        /* every other function takes a student name */
        if (!fgets (name1, sizeof name1, stdin)) {
            fprintf (stderr, "note: user canceled input.\n");
            return 1;
        }
        rmcrlf (name1);

        if (strcmp (order, "add") == 0)          /* use if, else if, else logic */
            add (&startptr, name1);
        else if (strcmp (order, "out") == 0)
            out (&startptr, name1);
        else if (strcmp (order, "first") == 0)
            first (&startptr, name1);
        else if (strcmp (order, "place") == 0) {      /* place takes two names */
            printf ("after: ");
            if (!fgets (name2, sizeof name2, stdin)) {       /* get name2 here */
                fprintf (stderr, "note: user canceled input.\n");
                return 1;
            }
            rmcrlf (name2);
            place (startptr, name1, name2);
        }
        else    /* handle the Bad Input case */
            fprintf (stderr, "error: invalid order, try again.\n");
    }

    return 0;
}

/** remove newline or carriage-return from 's'.
 *  returns new length. 's' must not be NULL.
 */
size_t rmcrlf (char *s)
{
    char *p = s;

    if (!*s)        /* s is empty-string */
        return 0;

    /* find eol or nul-terminating char */
    for (; *p && *p != '\n' && *p != '\r'; p++) {}

    if (*p == '\n' || *p == '\r')   /* validate eol & overwrite */
        *p = 0;
    else                            /* warn - no end-of-line */
        fprintf (stderr, "rmcrlf() warning: no eol detected.\n");

    return (size_t)(p - s);
}

int isempty (line *lptr) 
{
    return (lptr == NULL);
}

void backtoclassroom (line *currentptr) 
{
    printf ("\nline returning to classroom:\n\n");
    if (isempty (currentptr)) {
        printf ("line is empty.\n");
    }
    else {
        while (currentptr != NULL) {
            line *victim = currentptr;          /* ptr to node to free */
            printf ("  %s\n", currentptr->name);
            currentptr = currentptr->nextptr;
            free (victim);                      /* free your memory! */
        }
    }
}

static void reverse (line **lptr) 
{
    line *previousptr = NULL,
        *currentptr = *lptr,
        *afterptr;

    while (currentptr != NULL) {
        afterptr  = currentptr->nextptr;  
        currentptr->nextptr = previousptr;   
        previousptr = currentptr;
        currentptr = afterptr;
    }
    *lptr = previousptr;
}

void out (line **lptr, char *name) 
{
    line *iter = *lptr,
        *prev  = *lptr;

    if (strcmp ((*lptr)->name, name) == 0) {    /* name is first node */
        iter = iter->nextptr;                   /* save pointer to next */
        free (*lptr);                           /* free first */
        *lptr = iter;                           /* set first = saved */
        return;
    }

    while (iter && strcmp (iter->name, name)) { /* find node with name */
        prev = iter;                            /* save previousptr */
        iter = iter->nextptr;
    }
    if (!iter) {    /* handle name not found */
        fprintf (stderr, "error: %s not in list - can't remove.\n", name);
        return;
    }

    prev->nextptr = iter->nextptr;      /* previousptr = nextptr */
    free (iter);                        /* free current */
}

void first (line **lptr, char *name) 
{
    line *newptr = malloc (sizeof *newptr);     /* set size on current *ptr */
    strcpy (newptr->name, name);
    newptr->nextptr = *lptr;
    *lptr = newptr;
}

void place (line *lptr, char *name1, char *name2) 
{
    line *iter = lptr;
    line *newptr = malloc (sizeof *newptr);
    strcpy (newptr->name, name1);
    newptr->nextptr = NULL;

    while (iter && strcmp (iter->name, name2))  /* locate after: name */
        iter = iter->nextptr;

    if (!iter) {    /* handle name2 not found */
        fprintf (stderr, "error: %s not in list - can't place %s.\n", 
                name2, name1);
        return;
    }

    newptr->nextptr = iter->nextptr;
    iter->nextptr = newptr;
}

void add (line **lptr, char *name) 
{
    line *newptr = malloc (sizeof *newptr);     /* set size on current *ptr */
    line *lastptr = *lptr;
    strcpy (newptr->name, name);
    newptr->nextptr = NULL;
    if (*lptr == NULL) {
        *lptr = newptr;
        return;
    }  
    while (lastptr->nextptr != NULL) {
        lastptr = lastptr->nextptr;
    }
    lastptr->nextptr = newptr;    
}

Example Use/Output

$ ./bin/classline
orders [add, out, first, place, reverse, back to classroom]

order: add
name : john doe

order: add
name : mary smith

order: first
name : nancy first

order: place
name : sally second
after: nancy first

order: out
name : john doe

order: back to classroom

line returning to classroom:

  nancy first
  sally second
  mary smith

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to insure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/classline
==22574== Memcheck, a memory error detector
==22574== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==22574== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==22574== Command: ./bin/classline
==22574==
orders [add, out, first, place, reverse, back to classroom]
<snip>
line returning to classroom:

  nancy first
  sally second
  mary smith
==22574==
==22574== HEAP SUMMARY:
==22574==     in use at exit: 0 bytes in 0 blocks
==22574==   total heap usage: 4 allocs, 4 frees, 448 bytes allocated
==22574==
==22574== All heap blocks were freed -- no leaks are possible
==22574==
==22574== For counts of detected and suppressed errors, rerun with: -v
==22574== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Look things over and let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Hi, Thank you so mush for your time and effort. I take your advice serious and change the code of the program entirely. But Can you fix my problem as I explained ? or Tip how to do that ? `place` should actually create a virtual node. **place john jack** should hold a place for **jack** behind **john** and then when user type **add jack** just after this **jack** join the line and placed after **john**. – john Jan 25 '18 at 07:31
  • Sure, you can do that, but it will add a bit of code (more than you may think). You will add the placeholder node with a fixed name like `"RESERVED_jack"`. The reason it is needed is `place` only takes the name to add and the person to add it behind. That means the next loop of entering `order:` will start again and then on any subsequent `add` you will have to scan the entire list using `strncmp` to check for `"RESERVED_"` before each name. Are you **really sure** that's what you want? It doesn't make a whole lot of sense -- but it is doable. – David C. Rankin Jan 25 '18 at 07:55
  • What happens if `jack` never joins the line? What do you do with the dummy node then? Do you modify every other function to check for and skip any name with `"RESERVED_"` prefixing the name? It gets quite cumbersome quick -- that is why I think it is unlikely that is really what you are supposed to do. It may even be better to add a "placeholder" member to your `struct` and set it to `1` for dummy nodes. That would make the checks easier than checking for prefixed strings. – David C. Rankin Jan 25 '18 at 08:01
  • Or better yet, don't have `place` create any node, create a second list with a struct holding `name1` and `name2` and have `place` add that struct to a to that list. Have a second function `put_in_place` (that does what my `place` does now). Then in `add` compare the name to all `name1s` in the new list of names and if a match is found pass the `name1` and `name2` to `put_in_place` and remove that node from the new list. (see why I say I don't think that is really what your task intended...) – David C. Rankin Jan 25 '18 at 08:13