Hash table on top of linked list

[cppnoob25]

So I implemented a linked list and now I want to place a hash table on top. Am I going in the right direction?

I purposely avoided templates just to make this simpler for now. And some things are hard coded as well just to reduce how big my main is.

#include <iostream>

using namespace std;

class List{

private:

    struct Node{
    int data;
    Node * next;
    };
    Node * head;
    Node * current;

public:
    List();
    ~List();
    void push_front(int x);
    void push_back(int x);
    int pop_front();
    int pop_back();
    void print();

};

List::List()
{
    head = nullptr;
}

List::~List()
{
    Node * temp;
    current = head;
    while(current != nullptr)
    {
        temp = current;
        current = current->next;
        delete temp;
    }

}

void List::push_front(int x)
{
    Node *n = new Node;
    n->data = x;

    if(head == nullptr)
    {
        head = n;
    }
    else
    {
        current = head;
        n->next = current;
        head = n;
    }
}

void List::push_back(int x)
{
    Node *n = new Node;
    n->data = x;
    n->next = nullptr;

    if(head == nullptr)
    {
        head = n;
    }
    else
    {
        current = head;
        while(current->next != nullptr)
        {
            current = current->next;
        }
        current->next = n;
    }

}

void List::print()
{
    current = head;

    if(head == nullptr)
    {
        cout << "list is empty!" << endl;
    }
    else
    {
        while(current->next!= nullptr)
        {
            cout << current->data << " ";
            current = current->next;
        }
        cout << current->data << endl;
    }

}

int List::pop_back()
{
    Node * temp;
    current = head;
    int data;

    if(head == nullptr)
    {
        cout << "Error" << endl;
        return 0;
    }

    else{
        while(current->next != nullptr)
        {
            temp = current;
            current = current->next;
        }
        if(head == current)
        {
            head = nullptr;
            data = current->data;
            delete current;
            return data;
        }

        temp->next = nullptr;
        data = current->data;
        delete current;
        return data;
    }
}

int List::pop_front()
{
    current = head;
    int data;


    if(head == nullptr)
    {
        cout << "list is empty pf" << endl;
        return 0;
    }
    else
    {
     head = head->next;
     data = current->data;
     delete current;
     return data;
    }

}

class HashT{

private:
    List object[10];
    int hash(int x)
    {
        return x %10;
    }

public:
    HashT();
    void insert(int x);

};

HashT::HashT()
{
}

void HashT::insert(int x)
{
    object[hash(x)].push_back(x);
}

int main()
{
    HashT one;
    one.insert(5);
}

[dhayden]

Node::next remains uninitialized after lines 52, 70, 79.

There is no need for current to be a member of class List. Just use local variables where you need it.

Unless you want to add a tail pointer, I'd get rid of push_back() and pop_back() because they are hugely expensive if the list is large.

You will probably need to add find() and remove() methods to class List to find a Node by value and delete a node by value.

[cppnoob25]

Thanks for the feedback dhayden. I coded this pretty quick and guess I missed a few things. You're also right on those methods getting expensive. I just didn't think about adding the new integers to the top of the hash table.

[andywestken]

ASIDE

Does Node * current; need to be a class member? Given the way it's used it might be better as a local variable.

(And some of you variables could be defined in tighter scope!)

Andy