Questo sito utilizza cookies solo per scopi di autenticazione sul sito e nient'altro. Nessuna informazione personale viene tracciata. Leggi l'informativa sui cookies.
Username: Password: oppure
C/C++ - C programma liste doppiamente concatenate
Forum - C/C++ - C programma liste doppiamente concatenate

Avatar
sceva95 (Normal User)
Newbie


Messaggi: 4
Iscritto: 30/01/2016

Segnala al moderatore
Postato alle 17:41
Sabato, 30/01/2016
salve a tutti, ho un problema con questo programma e non riesco a trovare una soluzione: quando lo lancio dopo aver completato il ciclo for per la seconda scanf va in loop e non mi da più nulla, cosa ho sbagliato?

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

struct list{
    int value;
    struct list *next;
    struct list *prec;
};
    
void print (struct list *li){
    while(li != NULL){
        printf("%i ", li->value);
        li = li->next;
    }
}

void destroy (struct list *l){
    if (l != NULL){
        struct list *c = l;
        struct list *p = NULL;
        while(c!= NULL){
            if ( p!= NULL)
                free(p);
        };
    };
}
    

int main (void){
    int n = 10;
    int i;
    struct list *p = (struct list * ) malloc(sizeof(struct list));
    struct list *tail;
    scanf("%d", &p->value);
    tail = p;
    while ( n != 1){
        tail->next = (struct list *) malloc(sizeof(struct list));
        tail = tail->next;
        scanf("%d", &tail->value);
        tail->prec = p;
        p = p->next;
        n--;
    };
    tail->next = NULL;
    print (p);
    destroy(p);
}

    
        

PM Quote
Avatar
Template (Member)
Pro


Messaggi: 177
Iscritto: 09/12/2015

Segnala al moderatore
Postato alle 20:41
Sabato, 30/01/2016
Guarda questa funzione:

Testo quotato

Postato originariamente da sceva95:
Codice sorgente - presumibilmente C++

  1. void destroy (struct list *l){
  2.         if (l != NULL){
  3.                 struct list *c = l;
  4.                 struct list *p = NULL;
  5.                 while(c!= NULL){
  6.                         if ( p!= NULL)
  7.                                 free(p);
  8.                 };
  9.         };
  10. }





Il ciclo continua finchè C non è NULL (e non lo sarà mai, visto che le assegni un valore presumibilmente non NULL che non cambia), e la funzione free() si attiva se P non è NULL (ma P è sempre NULL, perchè le assegni il valore NULL e poi non la modifichi più).


Tra parentesi, avresti potuto trovare tu stesso questi errori con un minimo di debug... ti consiglierei di concentrarti molto su quest'aspetto, perchè un buon debug svolto da chi è stato coinvolto nella scrittura del codice spesso è risolutivo ed utile (soprattutto in ambito didattico) più di qualsiasi "consiglio esterno" ;)

Ultima modifica effettuata da Template il 30/01/2016 alle 20:43
PM Quote
Avatar
sceva95 (Normal User)
Newbie


Messaggi: 4
Iscritto: 30/01/2016

Segnala al moderatore
Postato alle 20:33
Domenica, 31/01/2016
grazie per la correzione, proprio non lo avevo notato; comunque adesso il programma si chiude ma la funzione print non mi stampa la lista, c'è qualche errore nell'elaborazione della lista? magari ho sbagliato la parte di codice dove associo i puntatori a il valore precedente?

PM Quote
Avatar
Template (Member)
Pro


Messaggi: 177
Iscritto: 09/12/2015

Segnala al moderatore
Postato alle 23:39
Domenica, 31/01/2016
Il problema sta nel while subito prima della chiamata di print(): tu scrivi

Codice sorgente - presumibilmente C/C++

  1. while ( n != 1){
  2.         tail->next = (struct list *) malloc(sizeof(struct list));
  3.         tail = tail->next;
  4.         scanf("%d", &tail->value);
  5.         tail->prec = p;
  6.         p = p->next;
  7.         n--;
  8.     };



Ma quello spostamento di P (assolutamente inutile comunque, visto che per puntare alla fine della lista usi già TAIL) fa sì che, alla fine del ciclo, esso punti all'ultimo elemento della lista: praticamente, hai perso il tuo puntatore alla testa.

Una forma corretta del codice è:

Codice sorgente - presumibilmente C/C++

  1. while (n != 1){
  2.                 tail->next = (struct list *) malloc(sizeof(struct list));
  3.                 tail = tail->next;
  4.                 scanf("%d", &tail->value);
  5.                 tail->prec = p;
  6.                 n--;
  7.         };




Detto questo, rileggendo il tuo codice ho notato alcune cose cui dovresti fare attenzione:

1 - Tu utilizzi, in tutte le funzioni, i parametri come variabili "modificabili": per esempio, qui

Codice sorgente - presumibilmente C/C++

  1. void print(struct list *li){
  2.         while (li != NULL){
  3.                 printf("%i ", li->value);
  4.                 li = li->next;
  5.         }
  6. }



Usi il puntatore facendolo "scorrere" lungo la lista. Questo non è strettamente sbagliato, ma senz'altro non aiuta la leggibilità e favorisce una condotta potenzialmente rischiosa: in questo caso non succede nulla, perchè il puntatore è passato by value e dunque ad essere oggetto di modifica è solo il valore della sua "copia" inserita nello stack frame della funzione print(), ma se per caso andassi a modificare il valore di ciò a cui esso punta produrresti dei cambiamenti anche nella variabile originale (perchè andresti a modificare il dato effettivamente puntato), e non è detto che tu voglia questo... fidati, è un errore che può capitare, soprattutto quando si scrive tanto codice ;)
Sarebbe meglio, a mio avviso, definire una variabile puntatore locale cui assegnare il valore del parametro e da far "scorrere" al suo posto ;)

2 - La funzione main() è di tipo int, dunque deve ritornare un valore.

3 - Tutte le scanf che hai inserito non sono precedute da alcuna stampa su console... praticamente, un ipotetico utente del tuo programma deve immaginare che tu stia aspettando un suo cenno di vita, e deve anche immaginare che tu ti aspetti che lui inserisca un intero! :rotfl:

PM Quote
Avatar
TheDarkJuster (Member)
Guru^2


Messaggi: 1620
Iscritto: 27/09/2013

Segnala al moderatore
Postato alle 1:16
Lunedì, 01/02/2016
Dovresti aggiungere un controllo errori : malloc può ritornare NULL!
E con quel codice sarebbe un disastro!

Riferito al punto 3: no, scherzi, io adoro scoprire da me come funzionano i programmi :rotfl:

PM Quote
Avatar
sceva95 (Normal User)
Newbie


Messaggi: 4
Iscritto: 30/01/2016

Segnala al moderatore
Postato alle 10:25
Lunedì, 01/02/2016
grazie per l'aiuto, comunque non ho messo printf prima degli scanf perchè è un programma di esercizio e quindi sapevo cosa dovevo inserire; ovviamente se fosse un programma "per il pubblico" le avrei messe specificando anche; seconda cosa: potreste spiegarmi perchè il puntatore che fa scorrere la testa è sbagliato? Togliendolo poi tutti i valori sono comunque legati doppiamente anche al parametro prec o si collegano solo al primo? poi potresti scrivermi un esempio del creare un puntatore locale come hai detto al posto di usare quello passato? (giusto per avere un idea di come si fa in modo giusto)
Per il resto ora il programma funziona (credo ) quindi grazie mille per gli aiuti dati :)

PM Quote
Avatar
Template (Member)
Pro


Messaggi: 177
Iscritto: 09/12/2015

Segnala al moderatore
Postato alle 14:29
Lunedì, 01/02/2016
Testo quotato

Postato originariamente da sceva95:
potreste spiegarmi perchè il puntatore che fa scorrere la testa è sbagliato?



In senso meramente "grammaticale" non è scorretto (il C permette quell'istruzione)... lo è concettualmente: di fatto, tu effettui gli inserimenti aggiungendo i nodi alla fine della lista, quindi il nodo di testa non cambia mai, e non ha senso che tu faccia scorrere il suo puntatore, perchè di fatto lo mandi a puntare un nodo intermedio, che non corrisponde a quello di testa.
Ti faccio un esempio: se hai questa lista

|1|->|2|->|3|

Il tuo puntatore alla testa P punta al valore 1, giusto? Bene, se tu inserisci in coda un altro nodo (come fai nel tuo programma), hai la lista:

|1|->|2|->|3|->|4|

Dove, fai attenzione, il nodo di testa non è cambiato (e quindi, non ha senso far cambiare il "bersaglio" di P): è cambiato solo il nodo di coda.


Testo quotato

Postato originariamente da sceva95:
Togliendolo poi tutti i valori sono comunque legati doppiamente anche al parametro prec o si collegano solo al primo?


Analizza il codice dentro il while:

Punto 1: crei un nuovo nodo successivo alla coda attuale
Codice sorgente - presumibilmente Plain Text

  1. tail->next = (struct list *) malloc(sizeof(struct list));



Punto 2: sposti il puntatore alla coda in modo che punti al nuovo "ultimo nodo"
Codice sorgente - presumibilmente Plain Text

  1. tail = tail->next;



Punto 3 (che risponde alla tua domanda): imposti il campo PREC del tuo ultimo nodo in modo che punti al suo predecessore (il "vecchio" ultimo nodo).
Codice sorgente - presumibilmente Plain Text

  1. tail->prec = p;




Quindi, di fatto, vai a creare una lista del tipo (la rappresento in verticale "stile albero" e con soli 4 nodi, per comodità):

NODO 1: |value = 1; prec = NULL; succ = NODO 2|   <- p
NODO 2: |value = 2; prec = NODO 1; succ = NODO 3|
NODO 3: |value = 3; prec = NODO 2; succ = NODO 4|
NODO 4: |value = 4; prec = NODO 3; succ = NULL| <- tail

Che è una lista 2-linkata, proprio come volevi che fosse: ogni elemento punta sia al suo predecessore che al suo successore ;)


Testo quotato

Postato originariamente da sceva95:
poi potresti scrivermi un esempio del creare un puntatore locale come hai detto al posto di usare quello passato? (giusto per avere un idea di come si fa in modo giusto)



Attenzione: quello che ti ho proposto non è "IL MODO GIUSTO" in senso assoluto, perchè anche quello che hai fatto tu è ammissibile: semplicemente, io ti ho proposto un modo in generale meno rischioso, più lineare e più leggibile (soprattutto in casi in cui ti trovi a dover gestire molti dati - penso, per esempio, ad implementazioni dell'algoritmo di Er o di certi altri algoritmi ricorsivi) di gestire il rapporto "variabili/parametri" ;)
Comunque, a titolo di esempio cito la tua funzione di stampa: tu l'hai scritta così:

Codice sorgente - presumibilmente C/C++

  1. void print(struct list *li){
  2.             while (li != NULL){
  3.                     printf("%i ", li->value);
  4.                     li = li->next;
  5.             }
  6.     }




Ecco, io ti propongo di riscriverla nella forma:

Codice sorgente - presumibilmente C/C++

  1. void print(struct list *li)
  2.    {
  3.             struct list *temp = li;
  4.  
  5.             while (temp != NULL)
  6.            {
  7.                     printf("%i ", temp->value);
  8.                     temp = temp->next;
  9.             }
  10.     }



In questo modo, l'output non è cambiato affatto (ottieni comunque la stampa di tutti i valori), ma hai ottenuto una versione più chiara e leggibile del codice, dove:

- I parametri non cambiano significato in corso d'opera: LI puntava alla testa della lista all'inizio, e continua a puntarvi alla fine
- Una variabile locale appositamente definita si occupa di lavorare sul valore passato dal parametro

È molto più ordinata e leggibile di prima, non credi? ;)

Ultima modifica effettuata da Template il 01/02/2016 alle 14:34
PM Quote
Avatar
sceva95 (Normal User)
Newbie


Messaggi: 4
Iscritto: 30/01/2016

Segnala al moderatore
Postato alle 16:58
Lunedì, 01/02/2016
grazie mille per la risposta, ora penso di aver capito molto di più :)

PM Quote