-2

I know this has something to do with eof, but I don't know how streams work exactly, i'd understand better if someone could tell me whats going on.

say I have 3 numbers {1, 2, 3} the load function puts the variables into the nodes, but when I go to print all the nodes only 1 will print.

void load() {
    ifstream fload;
    node *n = new node;
    node *temp = new node;
    fload.open("DoubleList.dat");
    if (fload) {
        fload >> n->data;
        n->next = NULL;
        n->prev = NULL;
        head = n;
        tail = n;
        curr = n;
        while (!fload.eof()) {
            fload >> temp->data;
            temp->next = NULL;
            temp->prev = curr;
            curr = temp;
            tail = temp;

        }
    }
}
Swordfish
  • 12,971
  • 3
  • 21
  • 43
devin
  • 368
  • 1
  • 3
  • 19
  • 1
    You don't ever seem to set a node's `next` to anything but `NULL`. The list will always appear to have at most 1 element. – François Andrieux Sep 26 '18 at 23:05
  • 1
    You should read [this question](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). – François Andrieux Sep 26 '18 at 23:06
  • I think you're right, might be a simple mistake. :( been looking at this for hours lol, Actually it doesn't need to point to anything I don't think. – devin Sep 26 '18 at 23:06
  • 1
    You only ever allocate 2 nodes so I'm not sure how you plan to store 3 values. There should be an allocation inside the loop somehow – M.M Sep 26 '18 at 23:11
  • When you find yourself looking for hours, stop.Consult your design documents to see where you may have gone wrong. If you have no design, why are you writing code? If you don't know what you are writing and why, you are almost always wasting your time. If you do manage to write code, how will you know if it works correctly without the program goals clearly outlined with testable metrics? – user4581301 Sep 26 '18 at 23:23
  • The reason shouldn't need to be explained, but if you must know I am trying to better my education in programming, and I believe practicing data models is a good tactic. As to why everyone loves to downvote what's wrong with you? what's wrong with my question? it was simple. I found out exactly where the issue was the leg work was done. I simply needed help with a programming problem, and everyone just throws negativity on this site. So amazing how much negativity is here. You hate over nothing. Just needed help on learning data structures. I appreciate the help. It also showed me different – devin Sep 27 '18 at 01:58
  • ways I could write the same code, and that I am very thankful for. as for the haters please unplug your computer – devin Sep 27 '18 at 01:59
  • The problem with this question is twofold: The question is lacking a [mcve]. I wouldn't downvote in this case because all of the information needed to answer the question was in the snippet you provided. This is uncommon, and, unfortunately, sets up the other likely downvote reason: The question was *too* simple. To some this means you didn't invest enough effort. But it often means your effort was wasted. I recommend spending time practicing logic, program design, and debugging as well as writing code.Start with a good plan and the program's much more likely to work. If not, debug. – user4581301 Sep 27 '18 at 05:55

2 Answers2

1

You are allocating only 2 nodes. If the file has less than 2 values, you leak memory. If the file has more than 2 values, you are not allocating a new node for every value.

Don't rely on eof(), either. Let operator>> tell you if it successfully read a value or not.

Try something more like this instead:

void load() {
    // TODO: make sure the list is freed and head/tail are null before continuing!

    ifstream fload;
    fload.open("DoubleList.dat");

    node **n = &head;
    T data; // <-- use whatever your actual node data type is...

    while (fload >> data) {
        *n = new node;
        (*n)->data = data;
        (*n)->next = NULL;
        (*n)->prev = tail;
        tail = *n;
        n = &(tail->next);
    }
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • 1
    Two notes for future readers about `node **n = &head;`. `head` is nothing but a node-less `next`. By abstracting the minor differences away you can use the same code for `head` and `next`, eliminating lots of nearly identical code. Second `n` contains a pointer to the last node's `next` (or `head`). This means you don't have to keep track of the last node to update its `next`. This saves you from needing extra variables and book-keeping code. – user4581301 Sep 27 '18 at 00:19
0
#include <fstream>

using namespace std;

struct node
{
    node *next;
    node *prev;
    double data;

    node(node *next, node *prev, double data)   // give node a constructor to not 
    : next{ next }, prev{ prev }, data{ data }  // clutter code with assignments
    {}
};

// assuming load is some member function of a list that has a head and a tail
void load() {
    ifstream fload{ "DoubleList.dat" };  // use constructor to open file
    if (!fload.is_open())
        return;  // early exit if file could not be opened

    node *root = nullptr;
    node *curr = root;

    double value;
    while (fload >> value)  // as long as doubles can be successfully extracted
    {
        if (!root) {  // if our list doesn't have a root yet
            curr = root = new node(nullptr, nullptr, value);
            head = tail = curr;
            continue;
        }

        curr->next = new node(nullptr, curr, value);  // construct the next node
        tail = curr = curr->next;  // and make it the current one.
    }
}
Swordfish
  • 12,971
  • 3
  • 21
  • 43