0

The program creates a hashtable from a csv. The function works only for the first piece of data, when I try to link a new node (linked chaining) it fails to attach a new node. I tried to read if there was a node present, and if there was to traverse until it found the end and then insert the node 'event' into the linked list.

Any help is appreciated.

struct hash_table_entry{
    int event_id;
    int year;
    int event_index;
    struct hash_table_entry *next = nullptr;
};

This is the function

bool insertHash(int index, hash_table_entry *event)
{
    if(hashtable[index] == nullptr)
    {
        hashtable[index] = event;
        return true;
    }
    hash_table_entry *pointingAt, *head;
    // before we move
    pointingAt = hashtable[index];
    head = hashtable[index];
    while(pointingAt->next)
    {
        pointingAt = pointingAt->next;
    }
    // insert
    cout <<  pointingAt << "<- thing inserted" << endl;
    pointingAt->next = event;
    return true;

}

This is how I implemented it in main (runs for each line in the csv)

insertHash(hashCalc(EVENT_ID,&c)
// c is the event
rcost
  • 23
  • 4
  • 1
    1) Separate the linked list from the hash table so you can implement and test separately. 2) Draw pictures. You know what the linked list should look like, so if you make a series to drawings showing the insertion process step by step, you can convert the drawings to code without much trouble. When debugging work through the code line by line and draw all of the instructions. If what you draw doesn't match the pictures, you know where you went wrong, and probably have a good idea of what you should do instead. – user4581301 Oct 21 '19 at 23:04
  • `while(pointingAt->next != 0)` and `if(pointingAt->next == nullptr)` what if `pointingAt` is null? – user4581301 Oct 21 '19 at 23:04
  • @user4581301 I assumed it would not be null because above that it checks if there is already a node present – rcost Oct 21 '19 at 23:13
  • 1
    `insertHash(hashCalc(EVENT_ID,&c)` Sets off a few alarmbells that I cannot confirm as being true or false positives., but it looks like you may be storing a pointer to a local variable. If this is in a loop, you might be using the same local variable over and over again. – user4581301 Oct 21 '19 at 23:14
  • @user4581301 yes it is in a loop, loops every line in a csv file. How would i get to use a new variable each run – rcost Oct 21 '19 at 23:18
  • Looks like you are. We can hack out some of that code. If you always test `pointingAt`, you can remove some redundancy. `hash_table_entry **pointingAt = &hashtable[index]; while(*pointingAt) { pointingAt = &pointingAt->next; } *pointingAt = event;` – user4581301 Oct 21 '19 at 23:19
  • To get a new event every time you'll need to use `new`. (or [smart pointers](https://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one), but I'm not a fan of smart pointers in linked lists. Easy to blow out the stack cleaning up a large list. – user4581301 Oct 21 '19 at 23:23
  • [If you want to go with smart pointers, Here's a really good video on how to do it.](https://www.youtube.com/watch?v=JfmTagWcqoE) – user4581301 Oct 21 '19 at 23:26
  • @user4581301 hash_table_entry* c = new hash_table_entry { stoi(EVENT_ID),stoi(YEAR), counter }; Is this correct use of new keyword? used in a loop for each line of a csv? – rcost Oct 21 '19 at 23:34
  • Looks right to me, and quick check pans out. Remember to `delete` all of these when you dispose of `hashtable`. – user4581301 Oct 21 '19 at 23:41
  • And watch out for [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – user4581301 Oct 21 '19 at 23:43
  • @user4581301 Thank you very much! one last thing, is there a reason that when i print what was inserted to cout, it gives me memory address. I apologize pointers easily trip me up hash_table_entry *pointingAt, *head; while(pointingAt->next) {pointingAt = pointingAt->next; } // insert cout << pointingAt << "<- thing inserted" << endl; pointingAt->next = event; – rcost Oct 21 '19 at 23:52
  • A pointer is nothing more than a variable that contains an address. The fact that you can use this address to get at data stored elsewhere is almost secondary. If you want to print out the data at the address, you need to dereference, `*pointingAt` and have an overload for `<<` that knows how to print a `hash_table_entry`. See [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) for some advice on how to write your own `operator<<` function. – user4581301 Oct 21 '19 at 23:58

1 Answers1

0

Helped by @user4581301 by adding the new keyword, in addition to changing up the way I append the node

hash_table_entry* c = new hash_table_entry
                {
            stoi(EVENT_ID),stoi(YEAR),
            counter
        };
rcost
  • 23
  • 4