Please look at my queue class and tell me what you think.

Please be gentle with me this is the first time in like 5 years that i've made any kind of serious effort at trying to learn pointers.

//node.h:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
#include <cstdio>
#ifndef node_h
#define node_h

template <class item>
struct node {
    node * next;
    item data;
    node(){ }
    ~node(){ }
};

#endif


#ifndef NODE_H
#define NODE_H

template <class item>
struct NODE {
    NODE * NEXTNODE;
    node<item> * nextnode;
};

#endif


#ifndef DEL
#define DEL

template <class type> bool del(type * n)
{
    if (n != NULL)
        {
            type * first; first = n;
            int counter = 0;
            while (n -> next != NULL)
                n = n -> next;
            delete n;
            if (first -> next != NULL)
                del (first);
            else delete first;
        }
    else return false;
}

template <class type> bool del (NODE<type> * N)
{
    if (N != NULL)
        {
            NODE<type> * FIRST; FIRST = N;
            while (N -> NEXTNODE != NULL)
                N = N -> NEXTNODE;
            delete N;
            if (FIRST -> NEXTNODE != NULL)
                del (FIRST);
            else delete FIRST;
            return true;
        }
    else return false;
}

#endif 


//myqueue.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
#ifndef MYQUEUE_H
#define MYQUEUE_H

#include "node.h"
#include <cstdlib>

template <class objT>
class myqueue {
    private:
    node<objT> * it; node<objT> * head;

    public:

    objT read() {if (it != NULL) return it -> data;}

    bool lineup(objT o)
        {
            if (it == NULL)
                {
                    it = new node<objT>;
                    it -> data = o;
                    it -> next = new node<objT>;
                    it -> next = NULL;
                    head = it;
                }
            else
                {
                    while (it -> next != NULL)
                        { it = it -> next; }
                    it -> next = new node<objT>;
                    it = it -> next;
                    it -> data = o;
                    it -> next = new node<objT>;
                    it -> next = NULL;
                    it = head;
                    head = it;
                }
            return true;
        }

    objT dequeue()
        {
            if (it != NULL)
                {
                    objT dat = read(); //it -> data;
                    discard();
                    return dat;
                }
            else return NULL;
        }

    int thesize()
        {
            if (it != NULL)
                {
                    int counter = 1;
                    node<objT> * T; T = it;
                    while (T -> next != NULL)
                        {  T = T -> next; ++counter;  }
                    it = head;
                    return counter;
                }
            return 0;
        }

    void discard(){
        if (it != NULL)
            {
                node<objT> * t;
                t = it -> next;
                it = t;
                head = it;
            }
        return;
        }

    myqueue()
        {
            it = new node<objT>;
            head = new node<objT>;
            head = it;
            it = NULL;
        }

    myqueue(const myqueue & rhs)
        {
            it = rhs;
            head = it;
        }

    myqueue(const objT & o)
        {
            it = new node<objT>;
            head = new node<objT>;
            it -> data = o;
            it -> next = NULL;
            head = it;
        }

    ~myqueue()
        {
            del(head);
            del (it);
        }
};
#endif


I havn't tested myqueue(const & rhs) yet. Am I right in assuming that I don't need to initialize it = new node<objT> because the node that they are being assigned to have already been allocated?

And I'm thinking about getting rid of the last constructor, I can't really think of any reason why it would be useful.

I guess I can get read of head too? I had previously use it for keeping track, and because I've seen other implementations with a similar function, I might decide to keep it for now.

Also please consider the following:
1
2
3
4
5
6
myqueue<int> list;
list.lineup(10);
list.lineup(25);
cout << "Removing: " << list.dequeue() << " and " << list.dequeue() << endl;
//cout << "Removing: " << list.dequeue() << endl;
//cout << "Removing: " << list.dequeue() << endl;  


The first cout statement prints the values in the reverse order...
Why?
Commment that line out and uncomment the other 2 cout statements and the values are couted in the correct order.

Thanks again for your help and your input.

Last edited on
I think that you should stop reinventing the wheel and just use the std::queue template class.

Here are a few comments.

1) your default constructor is counter intuitive. Why is it default constructing the first node? It ought to be constructing an empty queue.

2) The purpose of the copy constructor is to make an EXACT copy, nothing more nothing less. Yes you will have to dynamically allocate memory and copy each and every node otherwise you will not have properly copy constructed the object.

3) the documentation is terrible. I can't figure out what you are trying to do in many of the functions. Comments are a good thing.

4) Why are the del functions not part of the class?

5) I don't think that you can return NULL in the dequeue function.
Last edited on
The code contains memory leaks at several points. Lines 23, 31, and 34 are just three.
There are also some parts that, AFAICT, shouldn't compile, such as line 87.

EDIT:
5) I don't think that you can return NULL in the dequeue function.
You can, as long as objT can be constructed from the literal 0, as is the case with int. I any other case, the code will not compile.
Last edited on
so would this work to get rid of the memory leak from line 23:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
bool lineup(objT o)
        {
            if (it == NULL)
                {
                    it = new node<objT>;
                    it -> data = o;
                    it -> next = new node<objT>;
                    it -> next = NULL;
                    head = it;
                    delete it;
                    it = head;
                }
            else
                {
                    while (it -> next != NULL)
                        { it = it -> next; }
                    it -> next = new node<objT>;
                    it = it -> next;
                    it -> data = o;
                    it -> next = new node<objT>;
                    it -> next = NULL;
                    it = head;
                    head = it;
                }
            return true;
        }
Last edited on
No.
Topic archived. No new replies allowed.