6 Replies - 1625 Views - Last Post: 22 December 2004 - 10:30 PM Rate Topic: -----

#1 Videege  Icon User is offline

  • rÍvant.toujours
  • member icon

Reputation: 6
  • View blog
  • Posts: 1,413
  • Joined: 25-March 03

Linked List Issues

Posted 23 November 2004 - 08:12 PM

Ok, I am trying to implement a data storage system using a linked list, which, I must admit, I am pretty new at, and have been hacking around with it, so the following code may seem completely wrong to you, but I am blind to my own errors I suppose (runon sentence, meh). Ok, I have a function that loads float values from a text file (hence my last topic on displaying them in message boxes), but the real meat of the function is to put them into a linked list that is passed off as the parameter of the function. The relevant code is as follows:

bool _container::LoadData(_container *head)
{
	//open file and open linkedlist
	std::ifstream stream(head->file);

	MessageBox(NULL, "File Opened", "Filestream", MB_OK);
	if (!stream)
  return false;
	if (stream.is_open() == false)
  return false;

	char buffer[sizeof(stream) + 1];
	
	while (!stream.eof())
	{
  stream.getline(buffer, 49);
  static float price = atof(buffer);
  head->AddNode(price, head);
  
	}
	MessageBox(NULL, "while statement done", "Filestream", MB_OK);

	
	return true;
}

bool _container::AddNode(float price, _container *head)
{
	_container *tempa = new _container;
	_container tempb;
	tempb.data.price = price;
	tempa = head;
	tempb.file = head->file;
	while (tempa->next != NULL)
	{
  tempa = tempa->next;
	}
	tempb.next = NULL;
	tempa->next = new _stock;
	tempa->next->data.price = tempb.data.price;
	tempa->next->file = tempb.file;
	tempa->next->next = tempb.next;

	tempb.data.date.d = tempa->data.date.d++;
	//Update Date
	if (tempb.data.date.d >= 31)
	{
  tempb.data.date.d = 1;
  tempb.data.date.m++;
  if (tempb.data.date.m > 12)
  {
 	 tempb.data.date.y++;
 	 tempb.data.date.m = 1;
  }
	}
	tempa->next->data.date.d = tempb.data.date.d;
	tempa->next->data.date.m = tempb.data.date.m;
	tempa->next->data.date.y = tempb.data.date.y;
	
	return true;
}



You can probably ignore the whole updating date section in the AddNode() function, I'm just too lazy to cut it out. The result of running this function with a file "data.txt" with six values in it, the first of which being 43, is a linked list containing of six nodes, the first node's price value being 0 and the rest of them being 43. I've looked over this code and tried to modify it many times; I know I'm missing some fundamental logic step, but it seems like it should work to me. Thanks for any help.

EDIT:
BTW, I call the function in this manner:
_container *head = new _container;
head->LoadData(head);
...
_container *current = head;
while (current->next != NULL)
{
...output the price value of current...
current = current->next;
}
..
The text file looks like this:

43
83.9
.24
8547
63
22

This post has been edited by Videege: 23 November 2004 - 08:35 PM


Is This A Good Question/Topic? 0
  • +

Replies To: Linked List Issues

#2 malkiri  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 3
  • View blog
  • Posts: 364
  • Joined: 29-March 01

Re: Linked List Issues

Posted 24 November 2004 - 07:23 AM

A few points:
1. Your method of adding to the list is slightly off when the list is empty. The head pointer should be NULL when there's no data in the list. Your code assigns the head pointer a new _container object which doesn't have any data set. Instead, you should change:
while (tempa->next != NULL)
{
 tempa = tempa->next;
}

to
while (tempa != NULL)
{
 tempa = tempa->next;
}


And afterward, you operate on tempa itself, instead of tempa->next. This way, when the list is empty and the head pointer is NULL, you do all your operations on the actual head.

2. You have a memory leak - in the first line of AddNode, you allocate a new _container. Shortly afterward, you set tempa to point at the head pointer. You don't need to do that allocation.

2. "tempa" and "tempb" aren't very descriptive names. Something more like "currentNode" would be helpful, mostly because it's difficult to distinguish between 'tempa' and 'tempb' at a quick glance.

3. tempb isn't needed. There's never a point where you actually assign the object itself to anything - you just use it as a sort of intermediary container for values. You can just use the values themselves.

4. Remember that the post- and pre-increment operators not only return the value + 1, but actually set the value + 1. "tempb.data.date.d = tempa->data.date.d++;" has the side effect of incrementing tempa->data.date.d. It doesn't truly matter since you reassign it anyway, but that won't always be the case.

That should take care of the bug where you have a node with price 0 at the start. I can't locate the source of the other bug. Try updating your code and posting again. I'm in the dark about the relationship between _container and _stock, however. Can you describe it?
Was This Post Helpful? 0
  • +
  • -

#3 Videege  Icon User is offline

  • rÍvant.toujours
  • member icon

Reputation: 6
  • View blog
  • Posts: 1,413
  • Joined: 25-March 03

Re: Linked List Issues

Posted 24 November 2004 - 03:12 PM

First off, thanks for your help. I implemented a lot of your suggestions, but now I have a new problem..let me begin by posting the cleaned up code:

void test()
{
char buffer[50];
_container *head = new _container;

head->file = "data\data.txt";
head->LoadData(head);
_container *current = head;
while (current != NULL)
{
sprintf(buffer, "Value:      %f", current->data.price);
MessageBox(NULL, buffer, "Current->data.price", MB_OK);
current = current->next;
}
}

bool _container::LoadData(_container *head)
{
	//open file and open linkedlist

	std::ifstream stream(head->file);

	MessageBox(NULL, "File Opened", "Filestream", MB_OK);
	if (!stream)
  return false;
	if (stream.is_open() == false)
  return false;
	
	char buffer[sizeof(stream) + 1];

	while (!stream.eof())
	{
  stream.getline(buffer, 49);
  static float price = atof(buffer);
	
  MessageBox(NULL, buffer, "Filestream", MB_OK);
  head->AddNode(price, head);
  
	}

	return true;
}

bool _stock::AddNode(float price, _container *head)
{
	_container *currentNode = head;
	_container *referenceNode = NULL;
	while (currentNode != NULL)
	{
  referenceNode = currentNode;
  currentNode = currentNode->next;
	}
	currentNode = new _container;
	currentNode->next = NULL;
	currentNode->data.price = price;
	if (referenceNode)
       {
  currentNode->file = referenceNode->file;
                currentNode->data.date.d = referenceNode->data.date.d + 1;
       }
	else
       {
  currentNode->file = head->file;
                currentNode->data.date.d = head->data.date.d;
        }

	
	//Update Date
	if (currentNode->data.date.d >= 31)
	{
  currentNode->data.date.d = 1;
  currentNode->data.date.m++;
  if (currentNode->data.date.m > 12)
  {
 	 currentNode->data.date.y++;
 	 currentNode->data.date.m = 1;
  }
	}
	
	return true;
}



I think that about does it. The _stock type you saw in my last example was a typo when I wrote the post, it was supposed to be _container. I've added the referenceNode in the AddNode() function as a sort of last pointer that I can use to assign values of the previous node to the new node; tell me if I'm doing this the wrong way (I know you can simply implement a *previous pointer in the class, but meh ;P) When I run the test() function, the only value I recieve in the Message box is "Value: 000000.0", then the while statement finishes and the function exits. I don't know what's wrong with this; am I not resetting the list correctly when I read through it or am I making it in the wrong way?

EDIT: I put in a message box to read me the value of the buffer I am recieving from the text file; I get all the numbers in succession, so I know that the file is not the problem. Now if only I could get the MessageBox in the test() function to do the same, I would be happy :\...

This post has been edited by Videege: 24 November 2004 - 03:18 PM

Was This Post Helpful? 0
  • +
  • -

#4 Videege  Icon User is offline

  • rÍvant.toujours
  • member icon

Reputation: 6
  • View blog
  • Posts: 1,413
  • Joined: 25-March 03

Re: Linked List Issues

Posted 25 November 2004 - 01:46 PM

Anyone have any idea what's going on? I'm short on time, so I'm about to scratch this whole system and just write a crappy semi-dynamic array class.
Was This Post Helpful? 0
  • +
  • -

#5 Videege  Icon User is offline

  • rÍvant.toujours
  • member icon

Reputation: 6
  • View blog
  • Posts: 1,413
  • Joined: 25-March 03

Re: Linked List Issues

Posted 25 November 2004 - 07:19 PM

Well, I fixed it...turns out I was pointing to something wrong and my "same value" bug came from declaring the price variable as static...duh *hits head*. Thanks for your help, malkiri!
Was This Post Helpful? 0
  • +
  • -

#6 malkiri  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 3
  • View blog
  • Posts: 364
  • Joined: 29-March 01

Re: Linked List Issues

Posted 27 November 2004 - 08:13 AM

No problem. Sorry I didn't get back to you sooner - I've been away for Thanksgiving. :)
Was This Post Helpful? 0
  • +
  • -

#7 dlkj  Icon User is offline

  • D.I.C Head

Reputation: 4
  • View blog
  • Posts: 137
  • Joined: 11-March 02

Re: Linked List Issues

Posted 22 December 2004 - 10:30 PM

If this is not an assignment and you really want a good piece of software you can perhaps consider using the STL list container in <list>
This is assuming that you're going to use C++ of course.
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1