-2

I want to implement a circularly double linked list. this list just includes these chars in a passed in string object Here is my code, but I always get seg fault. i use a dummy head for this list

#ifndef MY_LIST_H
#define MY_LIST_H
#include <string>
#include <iostream>
using namespace std;
/**------   -----------------
 * dummy |->|pred|value|next|
 * ------   -----------------
 * */


struct Node
{
 char value;
 Node *next;
 Node *pred;

 Node( char value): value(value), next(0), pred(0){};
};

class MyList
{
    private:
    Node* head;
    unsigned int count; // count number of node

    public:
    // default constructor
    MyList(): count(0)
    {
        head = new Node('P');

    }

    //Constructs a list from a passed-in string object,
    MyList(const string& str): count(0)
    {
        Node *cur = head;
        if(count == 0)
        {
            head-> pred = head;
            head-> next = head;
        }
        else
        {
            for( unsigned i =0; i< str.length(); ++i)
            {
                cur->next = new Node(str.at(i));
                Node *temp = cur->next;
                temp-> pred = cur;
                ++count;
                if(count == str.length())
                {
                    cur->next->next = head;
                    head-> pred = cur-> next->pred;
                }
            }
        }
    }
    void print() const
    {
        Node *cur = head->next;
        while( cur != head)
        {
            cout << cur-> value;
            cur = cur-> next;
        }
    }

};
#endif
Paul Roub
  • 36,322
  • 27
  • 84
  • 93
Matt
  • 17
  • 1
  • 5

2 Answers2

1

You don't seem to understand constructors very well.

Only one constructor is called when you initialize your class. You can call a constructor from another constructor if you want, but that's not by defaut: Can I call a constructor from another constructor (do constructor chaining) in C++?.

In your instance, your second constructor should probably be something like this: MyList(const string& str): MyList() { ... }

That way head wil be properly initalized, and you won't create a segfault.

Additionnally you could run your code in debug mode, in the debugger, and find out the line your code crashes. using namespace ...; in a header is also a bad practice, as you don't know where your header will be included.

Community
  • 1
  • 1
coyotte508
  • 9,175
  • 6
  • 44
  • 63
0

It's hard to say exactly what's happening without see how you're using these classes but your MyList constructor overloaded on string is broken right off the bat. It sets count to 0 so you know it will always enter the if clause and never the else.

MyList(const string& str): count(0)
{
    Node *cur = head;
    if(count == 0)
    {
        head-> pred = head;
        head-> next = head;
    }
    else . . .

inside the if statement, it tries to dereference head which has never been assigned a value. You do set it in the default constructor but that one also doesn't seem to do anything else.

The purpose of a constructor is to construct a valid object from scratch. Sometimes one constructor overload might delegate to another to avoid repeated code but I'm not sure what you're trying to do here.

Assuming the second constructor was meant to actually be a helper method, well it still doesn't work because count never goes above zero (except in the else clause but you can't get there with count==0).

I'll admit I didn't look very carefully but I'm guessing that if execution this far:

cur->next->next 

is not always going to be set when you try to access it. if cur->next is nullptr then your program dies.

evan
  • 265
  • 2
  • 8