0

I have to read from file 100 task of a different kind in order to do different operation on a Queue, and print on an output file. But I get "terminate called after throwing an instance of 'std::bad_alloc' " As Error I think is something in template declaration. I tried to declare it in different ways but seems not to work. If someone can help me I'll be very gratefull.

#include    <iostream>
#include    <fstream>
#include    <string>

#define INPUT_FILE "input.txt"
#define OUTPUT_FILE "output.txt"
using namespace std;


template <typename T> class Pila
{
private:
bool empty=1;
public:
int lunghezza;
int cima;
T * Tab;
    Pila(int L){
        lunghezza=L;
        Tab= new T[L];
        cima=lunghezza;
    }
    void Push(T L){  
        cima--;
        Tab[cima]=L;
    }

    void Pop(){

        if(!empty)
        cima++;
        if(cima=lunghezza) empty=1;
    }

    bool Empty(){
       if(empty==1) return 1;
       else return 0;
    }

    T top(){
        return Tab[cima];
    }


    void Print(){
        for(int i=lunghezza-1; i>=cima; i--) cout<< Tab[i]<< " ";
    }
};

template <typename V> class Queue
{

public:
    int lunghezza;

    Queue(int l){
        lunghezza=l;
    };
    Pila<V> A= Pila <V>(lunghezza);
    Pila<V> B= Pila <V>(lunghezza);

    void enqueue(V L){ //sposta tutti gli elementi da A a B 
        while(!A.Empty()){ 
            B.Push(A.top());
            A.Pop();
        }

        A.Push(L); //Mette L dentro A

        while(!B.Empty()){   //sposta tutto di nuovo dentro A
            A.Push(B.top());
            B.Pop();
        }
    }


    void dequeue(){
        if(A.Empty()){
            cout<< " Coda Vuota"<< endl;
        }
        else       
        A.Pop();
    }    

    void Stampa(){
            A.Print();
            cout<< endl; 
    }
};

int main(){


    fstream infile, outfile;

    infile.open(INPUT_FILE, fstream::in);
    outfile.open(OUTPUT_FILE, fstream::out);


    int c=0, N,tmp;
    string tipo,operazione;
    while(c<100){
        infile>> tipo;
        infile>> N;
       if(tipo=="int"){
            Queue<int> A(N);
            for(int i=0; i<N; i++){
                infile>> operazione;
                if(operazione=="dequeue")
                A.dequeue();
                else
                {
                    int elem=stoi(operazione.substr(1));
                    A.enqueue(elem);
                }

            }
            A.Stampa();
        }

        if(tipo=="double"){
            Queue<double> A(N);
            for(int i=0; i<N; i++){
                infile>> operazione;
                if(operazione=="dequeue")
                A.dequeue();
                else
                {
                    double elem=stod(operazione.substr(1));
                    A.enqueue(elem);
                }

            }
            A.Stampa();
        }


        if(tipo=="bool"){
            Queue<bool> A(N);
            for(int i=0; i<N; i++){
                infile>> operazione;
                if(operazione=="dequeue")
                A.dequeue();
                else
                {
                    bool elem=stoi(operazione.substr(1));
                    A.enqueue(elem);
                }

            }
            A.Stampa();          
        }

        if(tipo=="char"){
            Queue<char> A(N);
            for(int i=0; i<N; i++){
                infile>> operazione;
                if(operazione=="dequeue")
                A.dequeue();
                else
                {
                    char elem=(char)(operazione[1]);
                    A.enqueue(elem);
                }

            }
           A.Stampa();           
        }


        c++;
    }

}
  • 1
    Off-topic: Get used to use English only identifiers in code. You *will* share code with people not knowing Italian language, and if only here on SO. Knowledge about what variables are used for will help them understanding your code better/faster and you might get better responses faster... – Aconcagua Jul 12 '19 at 09:33
  • 1
    You're leaking **_a lot_** of memory. I guess it runs out eventually, therefore `std::bad_alloc` is thrown. – Yksisarvinen Jul 12 '19 at 09:34
  • This is rather a lot of code to inspect and debug for you. Can you reduce it to a more minimal example? Maybe something that manipulates your Pila and Queue objects directly? – Botje Jul 12 '19 at 09:34
  • 1
    `std::bad_alloc` means, that memory allocation failed. Maybe you are trying to allocate too much memory? – churill Jul 12 '19 at 09:35
  • Side note: `empty` variable is redundant, you get the same information via `cima == lunghezza`. Try to avoid redundancies, they most often are a source of trouble. – Aconcagua Jul 12 '19 at 09:35
  • `if(cima=lunghezza)` – must be `if(cima == lunghezza)`, otherwise you do assignment (i. e. in concrete case, you empty the stack...). And you do not set `empty` to false on pushing (see: redundancies!). – Aconcagua Jul 12 '19 at 09:38
  • You have no check for stack being full already in `Pila::Push`. Only allow pushing, if cima is yet > 0! – Aconcagua Jul 12 '19 at 09:41
  • I eliminated "empty" changing the function. I also changed the Main in order to understand where's the problem, but same problem as before – Gianluca Di Mauro Jul 12 '19 at 09:44
  • 3
    `Pila(int L) { Tab= new T[L]; }` – if you allocate on creation, you need to destroy on destruction, i. e. you need a custom destructor as well. But then consider rules of [three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and [five](https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11)! Simpler solution: use a [smart pointer](https://en.cppreference.com/w/cpp/memory/unique_ptr) instead. – Aconcagua Jul 12 '19 at 09:45
  • Oh, and please: don't do `if(condition) return true; else return false;` – that's bad style, just do `return condition;`. If you use booleans, best just check them without comparisons (`if(empty)`). – Aconcagua Jul 12 '19 at 09:46
  • Honestly, your stack implementation (`Pila`) is not suitable to base a queue on. This copying around between those two stacks (`Queue::enqueue`) is inefficient as worse almost couldn't be. How did you get that idea? – Aconcagua Jul 12 '19 at 09:52
  • General recommendation: Don't write any output from within any general purpose classes (-> containers -> `Queue`). You impose that on any one using them even if not wanting (not at all or perhaps just in a different language), so she/he is trapped... (`cout << " Coda Vuota" << endl;`) – Aconcagua Jul 12 '19 at 09:55
  • I have to implement a template class Queue using only 2 Stacks. My idea was to fill a Stack and empty it into the other Stack – Gianluca Di Mauro Jul 12 '19 at 09:58
  • @GianlucaDiMauro I see, imposed by question/task... Still suggesting some improvement: On enqueuing, only shift all elements to stack B, but not back. Shift them back on dequeuing. This way, you only do shifts when really needed (sparing repeated shifts on subsequent enqueuings). – Aconcagua Jul 12 '19 at 10:04
  • `int c=0; while(c<100) { /* ... */ c++; }` is better written as `for(int c = 0; c < 100; c++) { /*...*/ }` – Aconcagua Jul 12 '19 at 10:08
  • About how much data in the files are we talking? You are running your loop 100 times, together with quite a large N and considering the memory leak (see my comment about rule of three/five and smart pointers), that might be an issue. – Aconcagua Jul 12 '19 at 10:11
  • Side note: Those `if(tipo == "XYZ") { }` all share the same code apart from minimal details. you could write a template function accepting two template parameters: type of queue and function to call to convert input. Then you'd call this function appropriately parametrised and would spare all this duplicated code... – Aconcagua Jul 12 '19 at 10:13
  • I have to read 100 input line from file; Each line can have up to 10000 elements.. – Gianluca Di Mauro Jul 12 '19 at 10:24

1 Answers1

1

Your first problem is in your initialization.

int lunghezza;

Queue(int l){
    lunghezza=l;
};
Pila<V> A= Pila <V>(lunghezza);
Pila<V> B= Pila <V>(lunghezza);

A and B will be initialized with the value of lunghezza which is not yet initialized. This way you can get a very high value that allocate lots of memory.

A better implementation of this part is to use initializers

int lunghezza;

Queue(int l) : lunghezza(l), A(l), B(l)
{
}

Pila<V> A;
Pila<V> B;

You second problem is in Pila class.

Pila(int L){
    lunghezza=L;
    Tab= new T[L];
    cima=lunghezza;
}

In this code, Tab is never disallocated so you will leak (loose) the allocated memory when your object will be destroyed. You must call delete[] Tab; in your destructor.

You can also use STL classes such as std::list or std::vector to replace your class Pila.

Cedric
  • 36
  • 5