0

My code its a double linked list of a products database, i have the options to input a product, search, delete, modify, show, save (.bin) and upload(.bin) All the options works before i use the load binary option. My problem is that when i upload the .bin with products the input a product option doesn´t and the delete a product does not work when i want to delete the last product in the list (all the others options works).

This is the input a product code:

void inP() {
    product* nuevo = new product();
    cout << "Ingrese el nombre del producto: ";
    cin >> nuevo->name;

    if (first == NULL) {
        first = nuevo;
        first->next = NULL;
        first->previous = NULL;
        last = first;
    }
    else {
        last->next = nuevo;
        nuevo->next = NULL;
        nuevo->previous = last;
        last = nuevo;
    }

    cout << "Producto agregado correctamente a la lista" << endl;
}

This is the delete a product code:

void deleteP() {

    product* current = new product();
    current = first;
    product* prev = new product();
    prev = NULL;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a eliminar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                cout << "\n" << current->name << " " << current->cant << " " << current->code << " " << current->marca << " " << current->descr << " " << current->monto << "\n";

                if (current == first) {
                    if (first->next == NULL) {
                        first = NULL;
                    }
                    else {
                        first = first->next;
                        first->previous = NULL;
                    }
                }
                else if (current == last) {
                    prev->next = NULL;
                    last = prev;
                }
                else {
                    prev->next = current->next;
                    current->next->previous = prev;
                
                }

                cout << "\n Producto Eliminado" << endl;
                found = true;
            }
            prev = current;
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

This is the load binary code:

void loadBin() {
    ifstream is(registryName, ios::in | ios::binary);
    product* reader;
    
    if (is.is_open()) {
        is.seekg(0, ios::end);
        int size = is.tellg();
        is.seekg(0, ios::beg);

        while (is.tellg() < size) {
            reader = new product;
            is.read((char*)reader, sizeof(product));
            reader->next = NULL;
            cout << reader->name << endl;
            
            if (first == NULL) {
                first = reader;
            }
            else {
                product* indice = first;
                while (indice->next != NULL) {
                    indice = indice->next;
                }
                indice->next = reader;
            }
        }
        is.close();
    }
}

And all the code:

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

using namespace std;

string registryName = "products.bin";

struct product {
    string name;
    product* next;
    product* previous;
}*first, * last;

void inP();
void outP();
void searchP();
void modifyP();
void deleteP();
void saveBin();
void loadBin();

int main() {

    
    int op;
    do {
        system("CLS");
        cout << "Menu:" << endl;
        cout << "1. Input a Product " << endl;
        cout << "2. Show the list " << endl;
        cout << "3. Search a Product " << endl;
        cout << "4. Modify a product " << endl;
        cout << "5. Delete a product " << endl;
        cout << "6. Save List " << endl;
        cout << "7. Load Binary List " << endl;
        cout << "8. Exit" << endl;
        cin >> op;
        cout << "\n";

        switch (op) {
        case 1: {
            inP();
            cout << "\n";
            system("PAUSE");
            break;
        }
        case 2: {
            outP();
            system("PAUSE");
            break;
        }
        case 3: {
            searchP();
            system("PAUSE");
            break;
        }
        case 4: {
            modifyP();
            system("PAUSE");
            break;
        }
        case 5:
        {
            deleteP();
            system("PAUSE");
            break;
        }
        case 6: {
            saveBin();
            break;
        }
        case 7: {
            loadBin();
            break;
        }
        case 8: {
            return 0;
        }
        default: cout << "No ingreso una opcion disponible" << endl;
            break;
        }
    } while (op!=8);

    return 0;
}

void inP() {
    product* nuevo = new product();
    cout << "Ingrese el nombre del producto: ";
    cin >> nuevo->name;

    if (first == NULL) {
        first = nuevo;
        first->next = NULL;
        first->previous = NULL;
        last = first;
    }
    else {
        last->next = nuevo;
        nuevo->next = NULL;
        nuevo->previous = last;
        last = nuevo;
    }

    cout << "Producto agregado correctamente a la lista" << endl;
}

void outP() {
    product* current = new product();
    current = first;

    if (first != NULL) {

        while (current != NULL) {
            cout << "\n" << current->name;
            current = current->next;
        }
    }
    else {
        cout << "No hay productos en la lista" << endl;
        cout << "\n";
    }
    cout << "\n" << endl;
}

void searchP() {

    product* current = new product();
    current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a buscar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void modifyP() {

    product* current = new product();
    current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a modificar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                cout << "\n Ingrese el nuevo nombre del Producto: ";
                cin >> current->name;
                cout << "\n Producto Modificado Correctamente \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void deleteP() {

    product* current = new product();
    current = first;
    product* prev = new product();
    prev = NULL;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a eliminar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;

                if (current == first) {
                    if (first->next == NULL) {
                        first = NULL;
                    }
                    else {
                        first = first->next;
                        first->previous = NULL;
                    }
                }
                else if (current == last) {
                    prev->next = NULL;
                    last = prev;
                }
                else {
                    prev->next = current->next;
                    current->next->previous = prev;
                
                }

                cout << "\n Producto Eliminado" << endl;
                found = true;
            }
            prev = current;
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void saveBin() {

    ofstream os(registryName, ios::out | ios::binary);

    if (os.is_open()) {
        product* indice = first;
        while (indice != NULL) {
            os.write((char*)indice, sizeof(product));
            indice = indice->next;
        }
        os.close();
    }

}

void loadBin() {
    ifstream is(registryName, ios::in | ios::binary);
    product* reader;
    
    if (is.is_open()) {
        is.seekg(0, ios::end);
        int size = is.tellg();
        is.seekg(0, ios::beg);

        while (is.tellg() < size) {
            reader = new product;
            is.read((char*)reader, sizeof(product));
            reader->next = NULL;
            cout << reader->name << endl;
            
            if (first == NULL) {
                first = reader;
            }
            else {
                product* indice = first;
                while (indice->next != NULL) {
                    indice = indice->next;
                }
                indice->next = reader;
            }
        }
        is.close();
    }
}

Josa Dmm
  • 9
  • 1
  • One cannot write a `std::string` to a binary file with the `write` function. A `std::string`, at its simplest, is a pointer to an array of `char` and a few book-keeping variables to track details like the size of the string. If you `write` that to the file, you write the pointer, **not the data the pointer points at**. You need to look up serialization. – user4581301 Feb 23 '22 at 02:44
  • [The answers here](https://stackoverflow.com/questions/10873382/write-and-read-string-to-binary-file-c) are almost right. They leave out a problem you aren't likely to see if your program reads and writes files and no one ever copies a file from one machine to another with a [different endian](https://en.wikipedia.org/wiki/Endianness) or integer size. Typically you use a [fixed width integer](https://en.cppreference.com/w/cpp/types/integer) to control the size and a function like [htonl](https://stackoverflow.com/questions/36924598/understanding-htonl-and-ntohl) to control endian. – user4581301 Feb 23 '22 at 02:51
  • The easiest way to handle this is to implement operators `<<` and `>>` for `product` and store everything as formatted text. If this is not an option, the output files MUST be binary, you should state this explicitly in the question. – user4581301 Feb 23 '22 at 02:55
  • Side note: Avoid stuff like `while (is.tellg() < size)` It's testing that everything is good before you read from the file, not that the read succeeded and you can use what you read. You always need to read, test that the read succeeded and then either use the data read or clean up the mess. If you try any other ordering you will have a bug. Test, read, use, risks using bad data. Ditto read, use, test. Any combination of use before read is an obvious non-starter. – user4581301 Feb 23 '22 at 02:59
  • At some point `while (indice->next != NULL) { indice = indice->next; }` will be dereferencing a NULL pointer. – AndersK Feb 26 '22 at 17:58
  • I think you should run this through the debugger. – AndersK Feb 26 '22 at 17:59

1 Answers1

0

Many many problems and misunderstandings . . .

First of all. I cannot see a linked list anywhere in your code. There is nowhere a statement or a variable with a list.

Instead, you have a Node called "product" and 2 global pointers to this data type. Then you manually link the Node instances together and update the global pointers correspondingly.

There is no "Segregation of Duties" and there are a lot of repetitions.

The normal approach would be to create a class or struct DoublyLinkedList, which would contain nodes and the first and last pointers. Then this class or struct would provide all interface functions that you need and hide all implementation internas from the oudsite world.

Then, for storing and saving. You would of course never store the pointers of the node "products" and cannot store a std::string as a binary. std::stringis a complex data type with some internal state invisible for you. This does not work.

Instead you need to do a serialization / deserialization of your data. This means that you just store the important data as a readable text.

The standard approach is to overwrite the insertion operator << and the extraction operator >> for your custom data type, here "product".

This can be done by simply adding

friend std::ostream& operator << (std::ostream& os, const product& p) { return os << p.name << '\n'; }
friend std::istream& operator >> (std::istream& is, product& p) { return std::getline(is, p.name); }

These statements (put into your struct definition) will overwrite the << and >> operator for your struct. Within the function, we simply write the "name" to the stream or read the "name" from the stream.

After having defined those overrides, we could write:

product p;
std::cin >> p;
std::cout << p;

The compiler will recognize data type "product" used with the >> or << operator and call the functions defined in your struct.


Additionally: You have some misunderstandings about new and pointers. You always create new products, even, if not needed.

For example in your "deleteP" function you write

    product* current = new product();
    current = first;
    product* prev = new product();
    prev = NULL;

Why do you create a new "product"? You just need to define a pointer.

So, the following approach must be used instead:

    product* current = first;
    product* prev = NULL;

And this leads us to the next and most severe problem in your code. You are not releasing the memory that you allocate with the function new. new will allocate/get memory via the operating system/stdandard-library. And all memory that you allocate, must be released again later with the delete function.

And since you do not delete anything, you are creating memory leaks like hell.

Everthing that is newed, must be deleted.

And basically new, delete and raw pointers for owned memory should not be used in C++.


But, one step after the other. First we fix the code with the minimum necessary correction (still with memory leaks). But adding de/serialization.

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

using namespace std;

string registryName = "products.bin";

struct product {
    string name;
    product* next;
    product* previous;
    friend std::ostream& operator << (std::ostream& os, const product& p) { return os << p.name << '\n'; }
    friend std::istream& operator >> (std::istream& is, product& p) { return std::getline(is, p.name); }
}*first, * last;

void inP();
void outP();
void searchP();
void modifyP();
void deleteP();
void saveBin();
void loadBin();

int main() {


    int op;
    do {
        system("CLS");
        cout << "Menu:" << endl;
        cout << "1. Input a Product " << endl;
        cout << "2. Show the list " << endl;
        cout << "3. Search a Product " << endl;
        cout << "4. Modify a product " << endl;
        cout << "5. Delete a product " << endl;
        cout << "6. Save List " << endl;
        cout << "7. Load Binary List " << endl;
        cout << "8. Exit" << endl;
        cin >> op;
        cout << "\n";

        switch (op) {
        case 1: {
            inP();
            cout << "\n";
            system("PAUSE");
            break;
        }
        case 2: {
            outP();
            system("PAUSE");
            break;
        }
        case 3: {
            searchP();
            system("PAUSE");
            break;
        }
        case 4: {
            modifyP();
            system("PAUSE");
            break;
        }
        case 5:
        {
            deleteP();
            system("PAUSE");
            break;
        }
        case 6: {
            saveBin();
            break;
        }
        case 7: {
            loadBin();
            break;
        }
        case 8: {
            return 0;
        }
        default: cout << "No ingreso una opcion disponible" << endl;
            break;
        }
    } while (op != 8);

    return 0;
}

void inP() {
    product* nuevo = new product();
    cout << "Ingrese el nombre del producto: ";
    cin >> nuevo->name;

    if (first == NULL) {
        first = nuevo;
        first->next = NULL;
        first->previous = NULL;
        last = first;
    }
    else {
        last->next = nuevo;
        nuevo->next = NULL;
        nuevo->previous = last;
        last = nuevo;
    }

    cout << "Producto agregado correctamente a la lista" << endl;
}

void outP() {
    product* current = new product();
    current = first;

    if (first != NULL) {

        while (current != NULL) {
            cout << "\n" << current->name;
            current = current->next;
        }
    }
    else {
        cout << "No hay productos en la lista" << endl;
        cout << "\n";
    }
    cout << "\n" << endl;
}

void searchP() {

    product* current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a buscar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void modifyP() {

    product* current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a modificar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                cout << "\n Ingrese el nuevo nombre del Producto: ";
                cin >> current->name;
                cout << "\n Producto Modificado Correctamente \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void deleteP() {

    product* current = first;
    product* prev = NULL;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a eliminar: ";
    cin >> searchP;

    if (first != NULL) {

        while (current != NULL && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;

                if (current == first) {
                    first = current->next;
                    first->previous = NULL;
                }
                else if (current == last) {
                    prev->next = NULL;
                    last = prev;
                }
                else {
                    prev->next = current->next;
                    current->next->previous = prev;
                }
                cout << "\n Producto Eliminado" << endl;
                found = true;
            }
            prev = current;
            current = current->next;
        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void saveBin() {

    ofstream os(registryName);
    if (os) {
        product* indice = first;
        while (indice != NULL) {
            os << *indice;
            indice = indice->next;
        }
    }
}

void loadBin() {
    ifstream is(registryName );
    if (is) {
        std::string name;
        while (is >> name) {
            product *reader = new product;
            reader->name = name;
            reader->next = NULL;
            cout << reader->name << endl;

            if (first == NULL) {
                first = reader;
                first->next = NULL;
                first->previous = NULL;
                last = first;
            }
            else {
                last->next = reader;
                reader->next = NULL;
                reader->previous = last;
                last = reader;
            }
        }
    }
}

This code still has many problems. The biggest ones are the memory leaks. And, an additional one. You have no "clearP" function, which will remove all nodes. Such a "clearP" function must normally be called before any "loadBin" function. Otherwise, id there are already elements available, "loadBin" will append data from the disk at the end of the nodes.

So, we need to add a "clearP" function. And in the next refactoring step, we will also release also the allocated memory. This we will do in the "deleteP" function and by calling "clearP" at the end of main and in the "loadBin" function.

With this corrections we will get:

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

using namespace std;

string registryName = "products.bin";

struct product {
    string name;
    product* next;
    product* previous;
    friend ostream& operator << (ostream& os, const product& p) { return os << p.name << '\n'; }
    friend istream& operator >> (istream& is, product& p) { return getline(is, p.name); }
}*first{ nullptr }, * last{ nullptr };

void inP();
void outP();
void searchP();
void modifyP();
void deleteP();
void saveP();
void loadP();
void clearP();

int main() {

    int op = 0;
    do {
        op = 0;
        while (op == 0) {
            system("CLS");
            cout << "Menu:" << endl;
            cout << "1. Input a Product " << endl;
            cout << "2. Show the list " << endl;
            cout << "3. Search a Product " << endl;
            cout << "4. Modify a product " << endl;
            cout << "5. Delete a product " << endl;
            cout << "6. Save List " << endl;
            cout << "7. Load List " << endl;
            cout << "8. Exit" << endl;
            if (not (cin >> op)) {
                op = 0;
                cin.clear();
                cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // skip bad input)
            }
        }
        cout << "\n";

        switch (op) {
        case 1: {
            inP();
            cout << "\n";
            system("PAUSE");
            break;
        }
        case 2: {
            outP();
            system("PAUSE");
            break;
        }
        case 3: {
            searchP();
            system("PAUSE");
            break;
        }
        case 4: {
            modifyP();
            system("PAUSE");
            break;
        }
        case 5:
        {
            deleteP();
            system("PAUSE");
            break;
        }
        case 6: {
            saveP();
            break;
        }
        case 7: {
            loadP();
            break;
        }
        case 8: {
            return 0;
        }
        default: cout << "No ingreso una opcion disponible" << endl;
            break;
        }
    } while (op != 8);

    clearP();

    return 0;
}

void inP() {
    product* nuevo = new product();
    cout << "Ingrese el nombre del producto: ";
    cin >> nuevo->name;

    if (first == nullptr) {
        first = nuevo;
        first->next = nullptr;
        first->previous = nullptr;
        last = first;
    }
    else {
        last->next = nuevo;
        nuevo->next = nullptr;
        nuevo->previous = last;
        last = nuevo;
    }

    cout << "Producto agregado correctamente a la lista" << endl;
}

void outP() {
    product* current = new product();
    current = first;

    if (first != nullptr) {

        while (current != nullptr) {
            cout << "\n" << current->name;
            current = current->next;
        }
    }
    else {
        cout << "No hay productos en la lista" << endl;
        cout << "\n";
    }
    cout << "\n" << endl;
}

void searchP() {

    product* current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a buscar: ";
    cin >> searchP;

    if (first != nullptr) {

        while (current != nullptr && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void modifyP() {

    product* current = first;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a modificar: ";
    cin >> searchP;

    if (first != nullptr) {

        while (current != nullptr && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;
                cout << "\n Ingrese el nuevo nombre del Producto: ";
                cin >> current->name;
                cout << "\n Producto Modificado Correctamente \n" << endl;
                found = true;
            }
            current = current->next;

        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void deleteP() {

    product* current = first;
    product* prev = nullptr;
    product* toDelete = nullptr;
    bool found = false;

    string searchP;

    cout << "Ingrese el producto a eliminar: ";
    cin >> searchP;

    if (first != nullptr) {

        while (current != nullptr && found != true) {

            if (current->name == searchP) {
                cout << "\n Producto ( " << searchP << " ) Encontrado \n" << endl;

                if (current == first) {
                    toDelete = first;
                    first = current->next;
                    first->previous = nullptr;
                    delete toDelete;
                }
                else if (current == last) {
                    toDelete = last;
                    prev->next = nullptr;
                    last = prev;
                    delete toDelete;
                }
                else {
                    toDelete = current;
                    prev->next = current->next;
                    current->next->previous = prev;
                    delete toDelete;
                }
                cout << "\n Producto Eliminado" << endl;
                found = true;
            }
            prev = current;
            current = current->next;
        }
        if (!found) {
            cout << "\n Producto no encontrado \n" << endl;
        }
    }
    else {
        cout << "\n La lista de productos esta vacia \n" << endl;
    }
}

void saveP() {

    ofstream os(registryName);
    if (os) {
        product* indice = first;
        while (indice != nullptr) {
            os << *indice;
            indice = indice->next;
        }
    }
}

void loadP() {
    ifstream is(registryName );
    if (is) {
        clearP();
        string name;
        while (is >> name) {
            product *reader = new product;
            reader->name = name;
            reader->next = nullptr;
            cout << reader->name << endl;

            if (first == nullptr) {
                first = reader;
                first->next = nullptr;
                first->previous = nullptr;
                last = first;
            }
            else {
                last->next = reader;
                reader->next = nullptr;
                reader->previous = last;
                last = reader;
            }
        }
    }
}

void clearP() {

    product* current = first;
    product* next = nullptr;

    if (first != nullptr) {

        while (current != nullptr) {
            next = current->next;
            delete current;
            current = next;
        }
    }
    first = nullptr;
    last = nullptr;
}

Still not a real llinked list, but already working.

Unfortunately I cannot show you the some example implementations for real lists, because SO limits answers to a lengt of 30000 . . .

A M
  • 14,694
  • 5
  • 19
  • 44