Noobish problem

Hello, I am new here(but I have lurked off and on). I finally decided to post about this. I was trying to get used to using linked list type structures and was trying to make one, but I did something major wrong here.

Code:

---

#include <iostream.h>
class mylinky
{
 private:
 static mylinky *first;
 mylinky *next;
 char content;
 public:
 mylinky(char input='p')
 {
  if (first==0)
    {
      first=this;
      content=input;
      next=0;
    }
  else
    {
      mylinky *finger;
      finger=first;
      while(!(finger->next==0)){finger=finger->next;};
      first=this;
      next=0;
    };
 }
 void printContents(void)
 {
  mylinky *finger;
  finger=first;
  while(!(finger==0))
  {
  cout<<finger->content<<".";
  finger=finger->next;
  };
 }
};

int main (void)
{
 system("PAUSE");
}

---

As this is, it compiles fine. However, if I change main to

Code:

---

int main (void)
{
 mylinky test = mylinky('q');
 system("PAUSE");
}

---

I get [Linker error] undefined reference to `mylinky::first' about 5 times. I know I am probably doing something fairly wrong here. I was tempted not to post this at all, but I figured I would give it a shot.

First, welcome to the board and congratulations on asking a reasonable question, posting pertinent code, indenting your code, and using code tags to post code to the board. I'd say you're off to a good start!


I would step back from the code and think about linked lists a little bit more. I think of linked lists in C++ as a group of objects linked together. That means that the linked list is one object and each of the links is an object in and of itself. Thus, I would break your code into two different classes, one for the link objects and one for the list.

Code:

---

struct MyLink
{
  char data;
  MyLink * next;
};
 
class MyList
{
  public:
        MyLink * first;
        MyList() : first(NULL) {};
        ~MyList() { //code to release memory allocated by new operator in addLink;}
        void addLink(char);
};
 
MyList::addLink(char ch)
{
  //develop a new MyLink object to add to this list
  MyLink* newLink = new MyLink();
  newLink->data = ch;
  newLink->next = NULL;
 
  //code to add newLink to this list
  if(first == NULL)
        first = newLink;
  else
        //code to add newLink to this list if this list is not empty
}
 
int main()
{
  MyList mylist;
  myList.addlink('y');
}

---

In your code you are trying to do everything within a single class and getting your feet crossed.

You don't need semicolons on the end of functions, only classes.

Does this compile?

Code:

---

mylinky test('q');

---

> static mylinky *first;
You have first declared as a static member of mylinky, which means every instance of mylinky shares the same first. If this is not what you want, then remove the static:
mylinky *first;

Otherwise, in addition to declaring them, you must define any static members of your class at some point.

Code:

---

mylinky *mylinky::first = NULL;

---

I have gotten a lot further, and I appreciate the help. This is definately something I needed to do. Special thanks for the bit about initializing Static members, even though it no longer applies to this problem(although I would like to retry doing what I did the first time once I get this other way of doing it worked out.)

Code:

---

#include <iostream>
struct myNode
{
 char value;
 myNode * next;
};
class myNodeList
{
 private:
 myNode * first;
 myNode * index;
 public:
 myNodeList() : first(0),index(0){};
 ~myNodeList()
 {
  myNode * finger;
  finger=first;
  while(!(finger==0))
  {
  myNode * nullnext;
  nullnext=finger->next;
  delete finger;
  finger = nullnext;
  }
 }
 void addNode(char value)
 {
  myNode * newNode = new myNode();
  newNode->value = value;
  newNode->next = 0;
  if (first==0){first=newNode;}
  else
  {
  myNode * finger;
  finger=first;
  while(!(finger->next==0)){finger=finger->next;};
  finger->next = newNode;
  }
 }
 char eachNode(void)
 {
  char ret;
  if (index=0){index=first;};
  if (!(index=0))
  {
  ret = index->value;
  index=index->next;
  }
  else
  {return 0;};
 }
 char printNodeList(void)
 {
  myNode * finger;
  finger=first;
  while(!(finger==0)){printf(&finger->value);finger=finger->next;};
 }
 int nodeCount(void)
 {
  myNode * finger;
  int count;
  finger=first;
  while(!(finger==0)){++count;finger=finger->next;};
  return count;
 }
};
int main(void)
{
 myNodeList nodez;
 nodez.addNode('y');
 nodez.addNode('a');
 nodez.addNode('y');
 nodez.addNode('!');
 nodez.printNodeList();
 printf("%d",nodez.nodeCount());
 delete &nodez;
 system("PAUSE");
}

---

Seems pretty good so far, but nodeCount is returning huge numbers, any idea why it might be doing that? EDIT:Found it. Me and my silly simpler language experiances poisoning my brain :) I wasn't starting Count out with a value so it had some silly giant number in it from the get-go. Well, I suppose I still have the question about the destructor, so I'll leave it:)

EDIT:Oh, and while I am posting, is my destructor cleaning up completely or is it leaking?

>Seems pretty good so far, but nodeCount is returning huge numbers, any idea why it might be doing that?

> int count;
Because count's never initialized?

Thanks, figured that out a split second before I read your post. I need to get the assumptions beat out of me on stuff like that.

I need some constructive criticism, if any, on my destructor as I am not 100% clear on how they work.

>Oh, and while I am posting, is my destructor cleaning up completely or is it leaking?
At first glace it looks good. Perhaps someone else can verify it.

>on my destructor as I am not 100% clear on how they work.
As you probably know, they get called whenever an object goes out of scope (at the end of main() in your case). Or if the object is allocated with new, when delete is called on that object.

There's quite a few here that really know linked lists, and can probably comment on your destructor. It's possible it could be written more concisely, but certainly looks good to me. :)

> delete &nodez;
And looking at your main(), this is incorrect. Only use delete if the object is created with new. Your destuctor will be called at the end of main().

So if you had:

Code:

---

myNodeList *nodez;

 nodez = new myNodeList;

---

You would need a delete. But making this a pointer is entirely unnecessary.

Ah ok. Now that makes more sense with New and Delete. So what it boils down to is that my variable is actually an entire myNodeList object, and not just a type of pointer to it.

This is the current version of my code (sorry for such a long thread).

Code:

---

#include <iostream>

class myNodeList
{
 private:
 struct myNode
 {
  char value;
  myNode * next;
 };
 myNode * first;
 myNode * index;
 public:
 myNodeList() : first(0),index(0){}
 ~myNodeList()
 {
  myNode * finger;
  finger=first;
  while(!(finger==0))
  {
  myNode * nullnext;
  nullnext=finger->next;
  delete finger;
  finger = nullnext;
  }
 }
 void addNode(char value)
 {
  myNode * newNode = new myNode();
  newNode->value = value;
  newNode->next = 0;
  if (first==0){first=newNode;}
  else
  {
  myNode * finger;
  finger=first;
  while(!(finger->next==0)){finger=finger->next;}
  finger->next = newNode;
  }
 }
 char eachNode(void)
 {
  char ret;
  if (index=0){index=first;}
  if (!(index==0))
  {
  ret=index->value;
  index=index->next;
  }
  else
  {ret= 0;}
  return ret;
 }
 char printNodeList(void)
 {
  myNode * finger;
  finger=first;
  while(!(finger==0)){printf(&finger->value);finger=finger->next;};
 }
 int nodeCount(void)
 {
  myNode * finger;
  int count=0;
  finger=first;
  while(!(finger==0)){++count;finger=finger->next;}
  return count;
 }
};
int main(void)
{
 myNodeList nodez;
 nodez.addNode('y');
 nodez.addNode('a');
 nodez.addNode('y');
 nodez.addNode('!');
 nodez.printNodeList();
 printf("\n");
 for (int index=0;index<nodez.nodeCount();++index)
 {
  printf("%d.",nodez.eachNode());
 }
 printf("\n");
 system("PAUSE");
}

---

If the length of this thread or my asking questions as-I-go breaks any rules(I don't think it does, but it might) feel free to lock or delete this thread, otherwise I will probably have a few more questions as I go.

I am still tinkering with it but, does anyone have any idea why eachNode won't spit out anything but zero? I am trying to use eachNode to build up a sort of "FOR...EACH...NEXT" style where I can step and process each member, allow them to delete themselves, etc.

EDIT: Woops, it was an == != = problem :)

Code:

---

>  if (index=0){index=first;}

---

Notice anything? By the way, there's a way to turn on warnings about this on most compilers. I'm using Dev-C++ and haven't figured out how yet, but I'm sure there's a way.

EDIT: problem solved

Dev C++ here too. Let me know if you find out, and I'll do the same.

Well, I have another problem. As I said before, I am trying to build up to using a FOR...EACH...NEXT style, and I am taking steps in that direction here. It compiles with zero errors, but then it crashes when run.

Code:

---

#include <iostream>

class myNodeList
{
 private:
 struct myNode
 {
  char value;
  myNode * next;
 };
 myNode * first;
 myNode * index;
 public:
 myNodeList() : first(0),index(0){}
 ~myNodeList()
 {
  myNode * finger;
  finger=first;
  while(!(finger==0))
  {
  myNode * nullnext;
  nullnext=finger->next;
  delete finger;
  finger = nullnext;
  }
 }
 void addNode(char value)
 {
  myNode * newNode = new myNode();
  newNode->value = value;
  newNode->next = 0;
  if (first==0){first=newNode;}
  else
  {
  myNode * finger;
  finger=first;
  while(!(finger->next==0)){finger=finger->next;}
  finger->next = newNode;
  }
 }
 void eachNode(void)
 {
  if (index==0){index=first;}
  if (!(index==0)){index=index->next;}
 }
 char eachNode_getValue(void)
 {
  return index->value;
 }
 char printNodeList(void)
 {
  myNode * finger;
  finger=first;
  while(!(finger==0)){printf(&finger->value);finger=finger->next;};
 }
 int nodeCount(void)
 {
  myNode * finger;
  int count=0;
  finger=first;
  while(!(finger==0)){++count;finger=finger->next;}
  return count;
 }
};
int main(void)
{
 myNodeList nodez;
 nodez.addNode('y');
 nodez.addNode('a');
 nodez.addNode('y');
 nodez.addNode('!');
 nodez.printNodeList();
 printf("\n");
 for (int index=0;index<nodez.nodeCount();++index,nodez.eachNode())
 {
  printf("%c",nodez.eachNode_getValue());
 }
 printf("\n");
 system("PAUSE");
}

---

I like your FOR...EACH idea. Now you have two problems. One in eachNode():

Code:

---

> void eachNode(void)
> {
>  if (index==0){index=first;}
>  if (!(index==0)){index=index->next;}
> }

---

This should really be:

Code:

---

void eachNode(void)
 {
  if (index==0){index=first;}
  else {index=index->next;}
 }

---

Otherwise you skip the first node. ;)
And then there's your fancy for-loop. It's stepping one too far. Try instead:

Code:

---

for (int index=0;index<nodez.nodeCount();++index)
 {
  nodez.eachNode();
  printf("%c",nodez.eachNode_getValue());
 }

---

>printf(&finger->value);
And I believe this would be better written:
printf("%c",finger->value);
as printf() is expecting a null-terminated char array for the first parameter.

Code:

---

#include <iostream>

class myNodeList
{
 private:
 struct myNode
 {
  char value;
  myNode * next;
 };
 myNode * first;
 myNode * index;
 public:
 myNodeList() : first(0),index(0){}
 ~myNodeList()
 {
  myNode * finger;
  finger=first;
  while(!(finger==0))
  {
  myNode * nullnext;
  nullnext=finger->next;
  delete finger;
  finger = nullnext;
  }
 }
 void addNode(char value)
 {
  myNode * newNode = new myNode();
  newNode->value = value;
  newNode->next = 0;
  if (first==0){first=newNode;}
  else
  {
  myNode * finger;
  finger=first;
  while(!(finger->next==0)){finger=finger->next;}
  finger->next = newNode;
  }
 }
 void eachNode(void)
 {
  if (index==0){index=first;}
  else {index=index->next;}
 }
 char eachNode_getValue(void)
 {
  return index->value;
 }
 void printNodeList(void)
 {
  myNode * finger;
  finger=first;
  while(!(finger==0)){printf("%c",finger->value);finger=finger->next;};
 }
 int nodeCount(void)
 {
  myNode * finger;
  int count=0;
  finger=first;
  while(!(finger==0)){++count;finger=finger->next;}
  return count;
 }
};
int main(void)
{
 myNodeList nodez;
 nodez.addNode('y');
 nodez.addNode('a');
 nodez.addNode('y');
 nodez.addNode('!');
 nodez.printNodeList();
 printf("\n");
 for (int index=0;index<nodez.nodeCount();++index)
 {
  nodez.eachNode();
  printf("%c.",nodez.eachNode_getValue());
 }
 printf("\n");
 system("PAUSE");
}

---

Works just peachy.

Thanks for all the advice. The problem came because the FOR loop wasn't evaluating the eachNode along with the index, it was ignoring the part after the comma.

However, I would like to know if there is a way I can squeeze the call to eachNode() up into the third bracket in FOR. It just looks cleaner to me, don't ask :)

Well, you could do it like this. I don't know why, but I had to move the: int index
outside the for-loop.

Code:

---

int index;
 for (index=0,nodez.eachNode();index<nodez.nodeCount();++index,nodez.eachNode())

---

You're right, the problem wasn't calling eachNode() too many times, but that it wasn't initialized before printing the first node.

This might seem overly picky, but wouldn't putting the int declaration inside the for loop technically be slightly superior because of scope?

Yes, if you can get it to compile. For some reason I couldn't get it to compile with it declared within the loop. :confused: But this would certainly be preferable.

About an earlier question, you can turn on compiler warnings under Dev-C++ (gcc) with the -Wall option (under Tools -> Compiler Options).

Ok, I got the struct and class version working. All well and good on that, that will be good for what I am going to need it for. Now I am trying my other method just because I am interested, and because I am genuinely interested in messing around with this stuff for the sake of understanding, not just to get what I need right this second and cut out.

Code:

---

class Node {
 private:
 static Node * root; Node * next;
 char content;
 public:
 Node(char content)
 {
  if (root == 0)
  {
  root = this;
  next = NULL;
  }
  else
  {
  Node * finger = root;
  while(!(finger->next==0)){finger=finger->next;}
  finger->next=this;
  }
  this->content = content;
 }
};

---

Now I have a problem. Don't worry too much about the rest of the class, I just included that to make the context clear. What I want to do is make a class that will maintain just a single list of it's type of object. It lacks the flexibility of the way Swoopy showed me, but I am interested in it because I want this to be available when I don't require the separate structs(basically, when I don't need the ability for a program to have more than one list of the given object.)

If I do it this way

Code:

---

static Node * root; Node * next;

---

then it doesn't like root, because it is undefined. [Linker error] undefined reference to `Node::root'

However, if I attempt to

Code:

---

static Node * root = NULL; Node * next;

---

then it says
4 ...\main.cpp invalid in-class initialization of static data member of non-integral type `Node*'

This makes me think that it is not liking a class having a static self-pointer. Is there a way around this?

When using static member variables you declare it in the class declaration and initialize it outside the class variable:

Code:

---

//A.h
//////////
class A
{
  static A * first;
};

A::A* first = NULL;

---

Thanks! The current state of my code is:

Code:

---

class Node
{
 private:
 static Node * root;static Node * index;Node * next;
 char content;
 public:
 Node(char content)
 {
  if (root == NULL)
  {
  root = this;
  next = NULL;
  }
  else
  {
  Node * finger = root;
  while(!(finger->next==NULL)){finger=finger->next;}
  finger->next=this;
  }
  this->content = content;
 }
 static int Count(void)
 {
  Node * finger;int count = 0;
  finger=root;
  while(!(finger->next==NULL)){finger=finger->next;++count;};
 }
};
Node::Node * root = NULL;
Node::Node * index = NULL;

---

I am still getting two [Linker error] undefined reference to `Node::root' errors in Dev C++ when I try to construct a node. :rolleyes: Crazy, but I still think getting used to C++ will be more than worth it. Any idea why it's not liking this?

Code:

---

Node::Node * root = NULL;

---

Do you mean

Code:

---

Node::root = NULL;

---

The compiler seems to like that even less :D Still, thanks for the help anyhow.

Code:

---

Node::Node * Node::root = NULL;

---

Wow, that seems to compile, thanks