How to print cities correctly using recursive?

Question

Hmm.. I've an hard time to do this print recursively.. can someone fix my code? its very hard to understand how to print all direction of the city where the next city also have an all direction in it.

cityHead will be the center of all city(can be at middle or other) and pointer of array i use it for store the city after the citynode created. So the thing i want right now is the print recursively. First is the maincenter city print, and the it will check the other 4 direction. and if the first direction city->west got city on it it will print it first. until all city got print it will change into city->west and print city-west all 4 direction if there is any city at the city->west direction..

struct cities{
    char name[100];
    cities *e,*w,*s,*n;

}*cityHead,*city[100];

    void printcity(cities *cityHead,int e,int w,int s,int n){
     if(cityHead){

        if(cityHead->w != NULL && w != 1){
            printf("--");
            printf("West: %s\n",cityHead->w->name);
            printcity(cityHead->w,1,0,0,0);
        }else{
            printf("--");
            printf("West: None\n");
        }

        if(cityHead->e != NULL && e != 1){
            printf("--");
            printf("East: %s\n",cityHead->e->name);
            printcity(cityHead->e,0,1,0,0);

        }else{
            printf("--");
            printf("East: None\n");
        }

        if(cityHead->s != NULL && s != 1){
            printf("--");
            printf("South: %s\n",cityHead->s->name);
            printcity(cityHead->s,0,0,0,1);

        }else{
            printf("--");
            printf("South: None\n");
        }

        if(cityHead->n != NULL && n != 1){
            printf("--");
            printf("North: %s\n",cityHead->n->name);
            printcity(cityHead->s,0,0,1,0);

        }else{
            printf("--");
            printf("North: None\n");
        }
    }
    }

    cities * newcity(char name[])
    {
       cities *temp = (cities)malloc(sizeof(cities));

       strcpy(temp->name,name);
       temp->e = temp->n = temp->s = temp->w = NULL;
       return temp;
     }

And also tell us more about what you want to do with the printing. As is, the question is not clear. — Paul Ogilvie, Apr 09 '20 at 12:40
cities *newcity(char name[]){ cities *temp = (cities*)malloc(sizeof(cities)); strcpy(temp->name,name); temp->e = temp->n = temp->s = temp->w = NULL; return temp; } — Reve, Apr 09 '20 at 12:56
the thing i want to print is the city where we start from the center of the city, first it will print the center, and check 4 direction which is east,west,south,north, then it will print east: none if the direction got no city there, and print east: cityX if there is city, it will be done recursively — Reve, Apr 09 '20 at 12:57
Please edit your question to explain what you want (it is still not clear) and show the definition of `cities`, not how you allocate it. Also, show your first call of the function (e.g. in `main`). — Paul Ogilvie, Apr 09 '20 at 13:03
Also please provide an example for output how you want it and the corespoding input — Ackdari, Apr 09 '20 at 13:09
@Reve I edited your question to give the defintion of *newcity* you put in a remark, next time edit yourself your question rather than to use a remark. Out of that since the beginning it is requested to have the definition of *cities* (a *struct* very probably) and more material. Without we cannot help you — bruno, Apr 09 '20 at 13:29
@Jabberwocky may be the OP just does not understand what we ask for, few days ago I was going to be crazy (supposing I am not already) because I asked several times the definition of a function but for the OP 'definition' was not about the code but about a textual description of the goal :-) — bruno, Apr 09 '20 at 13:31
@Reve please [edit] your question and add all relevant information including code _there_ and not in comments.Especially show us the definition of the type `cities`. — Jabberwocky, Apr 09 '20 at 13:35
The malloc looks bad anyway - seems to try to cast a pointer to a struct:( — Martin James, Apr 09 '20 at 13:46
@MartinJames yes OP missed the "_*_", it was already the case in the remark giving definition of *newcity* . We are in C, the cast is useless on _malloc_ result — bruno, Apr 09 '20 at 13:53
Finally I think understood the problem, the OP can thank the confinement at least in France because of the covid19 :-) — bruno, Apr 09 '20 at 14:44

bruno · Accepted Answer · 2020-04-09T16:03:46.217

very probably a reasonable definition of cities (which was much better named City, no reason so have it plural) is

typedef struct cities {
  char name[64];
  struct cities * e;
  struct cities * n;
  struct cities * s;
  struct cities * w;
} cities;

and your problem is to write all the links without entering in an infinite loop

A first thing to do is to immediately modify your program because you indicate the west etc of a city but you do not indicate for which city, so for instance :

  if(cityHead->w != NULL && w != 1){
      printf("--");
      printf("%s West: %s\n", cityHead->name, cityHead->w->name);
      printcity(cityHead->w,1,0,0,0);
  }else{
      printf("--");
      printf("%s West: None\n", cityHead->name);
  }

and so on for the other cases

Let see with 2 cities with the change :

int main()
{
  cities * c1 = newcity("c1");
  cities * c2 = newcity("c2");

  c1->e = c2;
  c2->w = c1;

  printcity(c1, 0, 0, 0, 0);
}

Execution :

pi@raspberrypi:/tmp $ ./a.out
--c1 West: None
--c1 East: c2
--c2 West: None
--c2 East: None
--c2 South: None
--c2 North: None
--c1 South: None
--c1 North: None

and we see the program does not indicate the east of c2 is c1

The reason is simple, for instance in

 if(cityHead->w != NULL && w != 1){
     printf("--");
     printf("%s West: %s\n", cityHead->name, cityHead->w->name);
     printcity(cityHead->w,1,0,0,0);
 }else{
     printf("--");
     printf("%s West: None\n", cityHead->name);
 }

w is clearly used to not enter in an infinite, but it also blocks the printf

So let modify it to have :

  if(cityHead->w != NULL){
      printf("--");
      printf("%s West: %s\n", cityHead->name, cityHead->w->name);
      if (w != 1)
        printcity(cityHead->w,1,0,0,0);
  }else{
      printf("--");
      printf("%s West: None\n", cityHead->name);
  }

There is also an error when you want to manage the north, the recursive call is :

printcity(cityHead->s,0,0,1,0);

but it must be

printcity(cityHead->n,0,0,1,0);

So finaly :

void printcity(cities *cityHead,int e,int w,int s,int n){
 if(cityHead){

    if(cityHead->w != NULL){
        printf("--");
        printf("%s West: %s\n", cityHead->name, cityHead->w->name);
        if (w != 1)
          printcity(cityHead->w,1,0,0,0);
    }else{
        printf("--");
        printf("%s West: None\n", cityHead->name);
    }

    if(cityHead->e != NULL){
        printf("--");
        printf("%s East: %s\n", cityHead->name,cityHead->e->name);
        if (e != 1)
          printcity(cityHead->e,0,1,0,0);

    }else{
        printf("--");
        printf("%s East: None\n", cityHead->name);
    }

    if(cityHead->s != NULL){
        printf("--");
        printf("%s South: %s\n", cityHead->name,cityHead->s->name);
        if (s != 1)
          printcity(cityHead->s,0,0,0,1);

    }else{
        printf("--");
        printf("%s South: None\n", cityHead->name);
    }

    if(cityHead->n != NULL){
        printf("--");
        printf("%s North: %s\n", cityHead->name,cityHead->n->name);
        if (n != 1)
          printcity(cityHead->n,0,0,1,0);

    }else{
        printf("--");
        printf("%s North: None\n", cityHead->name);
    }
 }
}

out of that let notice the initial test

if(cityHead){

is only useful if printcity is initially called with NULL, the function itself never recur with a NULL

Now the execution is :

pi@raspberrypi:/tmp $ ./a.out
--c1 West: None
--c1 East: c2
--c2 West: c1
--c2 East: None
--c2 South: None
--c2 North: None
--c1 South: None
--c1 North: None

If we have 4 cities making a square :

int main()
{
  cities * c1 = newcity("c1");
  cities * c2 = newcity("c2");
  cities * c3 = newcity("c3");
  cities * c4 = newcity("c4");

  c1->e = c2;
  c2->w = c1;

  c1->s = c3;
  c3->n = c1;

  c2->s = c4;
  c4->n = c2;

  c4->w = c3;
  c3->e = c4;

  printcity(c1, 0, 0, 0, 0);
}

the execution write a verrrrrrrry long times before to stop after too many recursions and a too large stack.

The reason is the int in parameters do not have enough extend in the graph.

A way to solve is to add a field in the struct to mark if the city was already managed. But out of modifying the struct that requests at the end to again go through the graph to clear the mark.

An other way is to save the already managed cities in a list, giving that list in parameter :

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

typedef struct cities {
  char name[64];
  struct cities * e;
  struct cities * n;
  struct cities * s;
  struct cities * w;
} cities;

typedef struct CityList {
  cities * city;
  struct CityList * next;
} CityList;

int isPresent(cities * c, CityList * l)
{
  while (l) {
    if (l->city == c)
      return 1;
    l = l->next;
  }
  return 0;
}

void add(cities * city, CityList ** l)
{
  CityList * cell = malloc(sizeof(CityList));

  cell->city = city;
  cell->next = *l;
  *l = cell;
}

void clear(CityList ** l)
{
  while (*l) {
    CityList * c = *l;

    *l = (*l)->next;
    free(c);
  }
}


void printcity(cities *cityHead, CityList ** l){
 if(cityHead && !isPresent(cityHead, *l)) {
    add(cityHead, l);
    if(cityHead->w != NULL){
        printf("--");
        printf("%s West: %s\n", cityHead->name, cityHead->w->name);
        printcity(cityHead->w, l);
    }else{
        printf("--");
        printf("%s West: None\n", cityHead->name);
    }

    if(cityHead->e != NULL){
        printf("--");
        printf("%s East: %s\n", cityHead->name,cityHead->e->name);
        printcity(cityHead->e, l);

    }else{
        printf("--");
        printf("%s East: None\n", cityHead->name);
    }

    if(cityHead->s != NULL){
        printf("--");
        printf("%s South: %s\n", cityHead->name,cityHead->s->name);
        printcity(cityHead->s, l);

    }else{
        printf("--");
        printf("%s South: None\n", cityHead->name);
    }

    if(cityHead->n != NULL){
        printf("--");
        printf("%s North: %s\n", cityHead->name,cityHead->n->name);
        printcity(cityHead->n, l);

    }else{
        printf("--");
        printf("%s North: None\n", cityHead->name);
    }
 }
}

cities * newcity(char name[])
{
   cities *temp = (cities*)malloc(sizeof(cities));

   strcpy(temp->name,name);
   temp->e = temp->n = temp->s = temp->w = NULL;
   return temp;
 } 


int main()
{
  cities * c1 = newcity("c1");
  cities * c2 = newcity("c2");
  cities * c3 = newcity("c3");
  cities * c4 = newcity("c4");
  CityList * l = NULL;

  c1->e = c2;
  c2->w = c1;

  c1->s = c3;
  c3->n = c1;

  c2->s = c4;
  c4->n = c2;

  c4->w = c3;
  c3->e = c4;

  puts("[]");
  printcity(c1, &l);
  clear(&l);

  /* to make a 8 */

  cities * c5 = newcity("c5");
  cities * c6 = newcity("c6");

  c3->s = c5;
  c5->n = c3;

  c4->s = c6;
  c6->n = c4;

  c5->e = c6;
  c6->w = c5;

  puts("8");
  printcity(c1, &l);
  clear(&l);

  free(c1);
  free(c2);
  free(c3);
  free(c4);
  free(c5);
  free(c6);

  return 0;
}

Execution :

pi@raspberrypi:/tmp $ ./a.out
[]
--c1 West: None
--c1 East: c2
--c2 West: c1
--c2 East: None
--c2 South: c4
--c4 West: c3
--c3 West: None
--c3 East: c4
--c3 South: None
--c3 North: c1
--c4 East: None
--c4 South: None
--c4 North: c2
--c2 North: None
--c1 South: c3
--c1 North: None
8
--c1 West: None
--c1 East: c2
--c2 West: c1
--c2 East: None
--c2 South: c4
--c4 West: c3
--c3 West: None
--c3 East: c4
--c3 South: c5
--c5 West: None
--c5 East: c6
--c6 West: c5
--c6 East: None
--c6 South: None
--c6 North: c4
--c5 South: None
--c5 North: c3
--c3 North: c1
--c4 East: None
--c4 South: c6
--c4 North: c2
--c2 North: None
--c1 South: c3
--c1 North: None
pi@raspberrypi:/tmp $

Note your definition of newcity is dangerous because of the line

strcpy(temp->name,name);

where you can write out of the field name in cities in case the name is too long, producing an undefined behavior

How to print cities correctly using recursive?

1 Answers1