[c++] funzione stampa di coda (implementata tramite lista)

Oo.Stud.ssa.oO
Ciao a tutti!! :)
C'è qualcosa che non va con la stampa della coda: stampa solo l'ultimo elemento inserito;
il programma è questo:
#include <cstdlib>
#include <iostream>
#define MAX 50

using namespace std;

typedef struct dati{
        char nome[MAX];
        char cognome[MAX];
        int eta;
        //costruttore di default
        dati(){
               nome[0] = '\0';
               cognome[0] = '\0';
               eta = 0;
               }
       //costruttore specifico
        dati(char _nome[MAX], char _cognome[MAX], int _eta){
                  strcpy(nome, _nome);
                  strcpy(cognome, _cognome);
                  eta = _eta;
                  }
        //metodo di stampa
        void stampa(){
             cout<<"nome:"<<nome<<" cognome:"<<cognome<<" eta':"<<eta<<endl;
             }
  };
  
  typedef struct nodo{
          dati data;
          nodo *next;
          
          nodo(dati d, nodo *n){
                    data = d;
                    next = n;}
          //metodo di stampa
         void stampa(){
               data.stampa();}
  };
  
  typedef struct TipoCoda{
          nodo *head;
          nodo *tail;
          TipoCoda(){
                     head = NULL;
                     tail = NULL;}
          //metodo di stampa
          void stampa(){
               nodo *s=head;
               while(s!=NULL){
                              s->stampa();
                              s=s->next;
                              }
               }
};
typedef TipoCoda Coda;
typedef TipoCoda *Codaptr;          


void put(Codaptr p, dati d);



la funzione put:
void put(Codaptr p, dati d){
     Codaptr n;
     n = p;
     nodo *q = new nodo(d,NULL);
     if(n->tail == NULL){//lista vuota
                n->head = q;}
     if(n->tail !=NULL){
                n->tail->next = q; //collego l'ultimo elemento al nuovo
                n->tail = q; //aggiorno tail
                p=n;}
   }}
                



Nel main:
Codaptr p = new Coda();
    dati d("Tania","Poletti",22);
    dati d2,d3("T","P",4);
    put(p,d);
    put(p,d2);
    put(p,d3);
    p->stampa();

Dove ho sbagliato?

Risposte
apatriarca
Il problema è a runtime o durante la compilazione? Se è un errore di compilazione posta l'errore, in caso contrario hai provato a fare il debug?

ulven101
Alla funzione put prova a passare Codaptr per riferimento: secondo me non aggiorna il puntatore.

apatriarca
Riguardando il codice sembrerebbe che ulven101 abbia ragione. Nella funzione put stai cercando di cambiare il valore del puntatore e hai bisogno di passarlo quindi per riferimento (usare insomma un puntatore a puntatore o un reference a puntatore).

ulven101
No, anzi, mi correggo, il problema non è quello.
Ti commento il codice.
void put(Codaptr p, dati d){
     Codaptr n;
     n = p;
     nodo *q = new nodo(d,NULL);
     /* Bene, ora fai il controllo che n->tail sia NULL. L'hai inizializzato così, quindi ti dirà sicuramente che la lista è vuota */
     if(n->tail == NULL){//lista vuota
                n->head = q;}

     /* Male: non entrerai mai in questo ciclo condizionato. Nel ciclo precedente non hai colto l'occasione per modificare n->tail, che resterà NULL */
     if(n->tail !=NULL){
                n->tail->next = q; //collego l'ultimo elemento al nuovo
                n->tail = q; //aggiorno tail
                p=n;}
   }}
                


n->head verrà sovrascritto ogni volta con l'ultimo elemento immesso. Quasi sicuramente è per questo che stampa solo l'ultimo elemento.

Oo.Stud.ssa.oO
"ulven101":
No, anzi, mi correggo, il problema non è quello.
Ti commento il codice.
void put(Codaptr p, dati d){
     Codaptr n;
     n = p;
     nodo *q = new nodo(d,NULL);
     /* Bene, ora fai il controllo che n->tail sia NULL. L'hai inizializzato così, quindi ti dirà sicuramente che la lista è vuota */
     if(n->tail == NULL){//lista vuota
                n->head = q;}

     /* Male: non entrerai mai in questo ciclo condizionato. Nel ciclo precedente non hai colto l'occasione per modificare n->tail, che resterà NULL */
     if(n->tail !=NULL){
                n->tail->next = q; //collego l'ultimo elemento al nuovo
                n->tail = q; //aggiorno tail
                p=n;}
   }}
                


n->head verrà sovrascritto ogni volta con l'ultimo elemento immesso. Quasi sicuramente è per questo che stampa solo l'ultimo elemento (non ho controllato quella funzione).

A...vero!!!!!!!
In pratica devo mettere anche
n->tail = q;
giusto?
Il primo ciclo quindi risulta
 if(n->tail == NULL){//lista vuota
                n->head = q;
                n->tail = q;}

Si adesso funziona...Grazie a TUTTI ragazzi!! :D :smt109 :smt109

apatriarca
Non c'è in realtà alcuna ragione di definire n all'interno di quella funzione.. Puoi usare direttamente p. Credo dovresti comunque usare nomi un po' più significativi. Difficile nella tua funzione capire immediatamente che cosa sia q e cosa siano n e p. Ad esempio: usando la lettera n si immagina che si tratti di un nodo, ma non è così..

ulven101
"apatriarca":
Non c'è in realtà alcuna ragione di definire n all'interno di quella funzione.. Puoi usare direttamente p. Credo dovresti comunque usare nomi un po' più significativi. Difficile nella tua funzione capire immediatamente che cosa sia q e cosa siano n e p. Ad esempio: usando la lettera n si immagina che si tratti di un nodo, ma non è così..


Per non parlare del fatto che così sprechi 1 B di memoria :P

apatriarca
Non capisco se è ironico e a cosa tu ti riferisca con lo spreco di un byte di memoria.. Il commento era esclusivamente legato alla leggibilità del codice. Variabili con nomi più chiari e significativi rendono più semplice la lettura e la comprensione del codice.

ulven101
No, non è ironico, nè sarcastico, anche se rileggendolo ammetto che è equivocabile.
Per sistemi a 8bit ogni puntatore richiede 1B per essere memorizzato, per sistemi a 32bit ne servono 4, per i sistemi a 64 bit ne servono ben 8.
Volevo sottolineare il fatto che del codice potenzialmente brutto ed illeggibile - anche se funziona - è spesso e volentieri fonte di spreco di risorse.

Oo.Stud.ssa.oO
Ma come è possibile, questo pomeriggio funzionava, adesso se inserisco un solo elemento continua a stamparlo all'infinito..invece se ne inserisco più di 1 funziona.. :|

ulven101
Puoi ripostare il codice aggiornato, per favore?

Oo.Stud.ssa.oO
Certo
strutture dati + header:
#include <cstdlib>
#include <iostream>
#define MAX 50

using namespace std;

typedef struct dati{
        char nome[MAX];
        char cognome[MAX];
        int eta;
        //costruttore di default
        dati(){
               nome[0] = '\0';
               cognome[0] = '\0';
               eta = 0;
               }
       //costruttore specifico
        dati(char _nome[MAX], char _cognome[MAX], int _eta){
                  strcpy(nome, _nome);
                  strcpy(cognome, _cognome);
                  eta = _eta;
                  }
        //metodo di stampa
        void stampa(){
             cout<<"nome:"<<nome<<" cognome:"<<cognome<<" eta':"<<eta<<endl;
             }
  };
  
  typedef struct nodo{
          dati data;
          nodo *next;
          
          nodo(dati d, nodo *n){
                    data = d;
                    next = n;}
          //metodo di stampa
         void stampa(){
               data.stampa();}
  };
  
  typedef struct TipoCoda{
          nodo *head;
          nodo *tail;
          TipoCoda(){
                     head = NULL;
                     tail = NULL;}
          //metodo di stampa
          void stampa(){
               nodo *s=head;
               while(s!=NULL){
                              s->stampa();
                              s=s->next;
                              }
               }
};
typedef TipoCoda Coda;
typedef TipoCoda *Codaptr;          

void put(Codaptr p, dati d);
dati get(Codaptr p);



funzione put:

void put(Codaptr p, dati d){
     Codaptr n;
     n = p;
     nodo *q = new nodo(d,NULL);
     if(n->tail == NULL){//lista vuota
                n->head = q;
                n->tail = q;}
     if(n->tail !=NULL){
                n->tail->next = q; //collego l'ultimo elemento al nuovo
                n->tail = q; //aggiorno tail
                p=n;}
   }
                


funzione get:
dati get(Codaptr p){
     dati d; //dato inizializzato con costruttore di default
     if(p->head == NULL){
                return d;
                }else{
                      d=p->head->data;
                      if(p->head->next == NULL){ //se ho un solo elemento
                                    p->head = NULL;
                                    p->tail = NULL;
                                     }else{
                                         p->head = p->head->next;
                                         return d;}
                                        }
  }  


nel main:

    Codaptr c = new Coda();
    dati d("1","1",1),d1;
    dati g;
    put(c,d);
    put(c,d1);
    c->stampa();
    g = get(c);
    cout<<", ora la lista:"<<endl;
    c->stampa();


In pratica: se inserisco 1 elemento e stampo cicla all'infinito. Se inserisco più di un elemento e stampo funziona.
Se inserisco 2 elementi, ne tolgo uno col get e stampo funziona.
Non capisco...

ulven101
L'errore è ancora nella funzione put ;-)
Per come sono strutturati i cicli, il primo elemento che viene immesso punta sè stesso.
Trova l'errore, se vuoi arrivarci da sola. Ti scrivo il perchè nello spoiler.
    void put(Codaptr p, dati d){
         Codaptr n;
         n = p;
         nodo *q = new nodo(d,NULL);
         if(n->tail == NULL){//lista vuota
                    n->head = q;
                    n->tail = q;}
         if(n->tail !=NULL){
                    n->tail->next = q; //collego l'ultimo elemento al nuovo
                    n->tail = q; //aggiorno tail
                    p=n;}
       }
                   



Soluzione

Oo.Stud.ssa.oO
"ulven101":
L'errore è ancora nella funzione put ;-)
Per come sono strutturati i cicli, il primo elemento che viene immesso punta sè stesso.
Trova l'errore, se vuoi arrivarci da sola. Ti scrivo il perchè nello spoiler.
    void put(Codaptr p, dati d){
         Codaptr n;
         n = p;
         nodo *q = new nodo(d,NULL);
         if(n->tail == NULL){//lista vuota
                    n->head = q;
                    n->tail = q;}
         if(n->tail !=NULL){
                    n->tail->next = q; //collego l'ultimo elemento al nuovo
                    n->tail = q; //aggiorno tail
                    p=n;}
       }
                   



Soluzione



Ho guardato la tua risposta :) in effetti mettendo un "else " tra i due "if", o anche solo scambiando di posto ai due "if" funziona!
Grazie infinite

ulven101
Ho guardato la tua risposta :) in effetti mettendo un "else " tra i due "if", o anche solo scambiando di posto ai due "if" funziona!
Grazie infinite


L'if-else è da preferire al semplice scambio, in questo modo il corretto funzionamento del codice dipende da una scelta strutturale dei controlli e non dall'ordine nel quale vengono eseguiti.
Sì, sono pignolo.

Oo.Stud.ssa.oO
:) si ma sei bravo

apatriarca
Non è pignoleria. A volte si riprendono in mano codici scritti parecchio tempo prima. Avere un qualche tipo di struttura o logica nascosta è raramente una buona idea in questi casi perché è facile dimenticarsene. Se all'esame ti chiedesse di implementare qualcosa di questo tipo e riprendessi in mano il codice, ti ricorderesti del legame tra i due if? Della motivazione per cui uno dei due va scritto prima dell'altro. Ne dubito e dubito che te ne ricorderesti poi all'esame con tutte le altre cose a cui pensare. Meglio un if/else..

Rispondi
Per rispondere a questa discussione devi prima effettuare il login.