patterncMinor
Hash map implementation
Viewed 0 times
implementationmaphash
Problem
I'm a student in college, and while I have coded a lot, I still feel like I've only scraped the surface. I have learned a lot of things over the years, but very few of my professors ever did code reviews and I feel like I may have some bad habits that I may be overlooking, one that was pointed out to me was that I didn't write any comments. I'd like to see if there are any others.
HashMap.h:
HashMap.c:
```
#include
#include "hashmap.h"
HashmapNode* hashmap_new_node(unsigned int hash_index,int values_size)
{
HashmapNode *hashmap_node = malloc(sizeof(HashmapNode));
hashmap_node->hash_index = hash_index;
hashmap_node->values_size = values_size;
hashmap_node->values = malloc(sizeof(values_size * sizeof(int)));
return hashmap_node;
}
HashMap* hashmap_new(int size)
{
int i;
HashMap hashmap = (HashMap)malloc(sizeof(HashMap));
hashmap->node_list = malloc(size sizeof(HashmapNode));
hashmap->element_count = 0;
hashmap->map_size = size;
for(i = 0; i node_list[i] = malloc(sizeof(HashmapNode));
hashmap->node_list[i] = NULL;
}
return hashmap;
}
unsigned int hashmap_hash_func(HashMap *hashmap, unsigned int key)
{
int hash = key;
hash = (hash >> 3) * 2654435761;
hash = hash % hashmap->map_size;
return hash;
}
void hashmap_expand(HashMap *hashmap)
{
HashMap.h:
#ifndef HASHMAP_H
#define HASHMAP_H
typedef struct HashmapNode{
unsigned int hash_index;
int value;
}HashmapNode;
typedef struct HashMap{
int element_count;
int map_size;
HashmapNode ** node_list;
}HashMap;
HashmapNode* hashmap_new_node(unsigned int hash_index, int * values);
HashMap* hashmap_new(int size);
unsigned int hashmap_hash_func(HashMap * hashmap, unsigned int key);
void hashmap_expand(HashMap *hashmap);
void hashmap_delete(HashMap *hashmap, int key);
void hashmap_insert(HashMap *hashmap, unsigned int key, int * values, int values_size);
HashmapNode* hashmap_lookup(HashMap *hashmap, unsigned int key);
void hashmap_destroy(HashMap *hashmap);
#endifHashMap.c:
```
#include
#include "hashmap.h"
HashmapNode* hashmap_new_node(unsigned int hash_index,int values_size)
{
HashmapNode *hashmap_node = malloc(sizeof(HashmapNode));
hashmap_node->hash_index = hash_index;
hashmap_node->values_size = values_size;
hashmap_node->values = malloc(sizeof(values_size * sizeof(int)));
return hashmap_node;
}
HashMap* hashmap_new(int size)
{
int i;
HashMap hashmap = (HashMap)malloc(sizeof(HashMap));
hashmap->node_list = malloc(size sizeof(HashmapNode));
hashmap->element_count = 0;
hashmap->map_size = size;
for(i = 0; i node_list[i] = malloc(sizeof(HashmapNode));
hashmap->node_list[i] = NULL;
}
return hashmap;
}
unsigned int hashmap_hash_func(HashMap *hashmap, unsigned int key)
{
int hash = key;
hash = (hash >> 3) * 2654435761;
hash = hash % hashmap->map_size;
return hash;
}
void hashmap_expand(HashMap *hashmap)
{
Solution
Always Test the Value Returned From Malloc
The memory allocation functions such as
Example of a possible correct way:
In C Always Initialize the Variables
In C there is no guarantee that a variable on the stack is going to
be initialized to zero or NULL. To avoid unknown behavior and unknown results initialize all variables when they are declared. This has saved me a lot of debugging over the years. When I was writing code that integrated many libraries this was one of the type of bugs I often had to track down.
Portability
The HashMap.c file contains
but it is named HashMap.h in the question. Some operating systems ignore case in file names, others are quite strict. To avoid portability issues make sure the name of the included file is the same for both the operating system and in the #include.
calloc() versus malloc()
The function calloc(size_t n_items, size_t size_of_one_location) is meant to allocate arrays of some item. It makes the code clearer with similar results.
The memory allocation functions such as
malloc() and calloc() may fail for a number of reasons, such as there is no memory to allocate or there is no block of memory large enough to allocate. When these memory functions fail they return NULL. After every call to malloc() always check the pointer receiving the memory address to see if malloc() returned NULL. This prevents accessing a memory address through a NULL pointer.Example of a possible correct way:
HashmapNode *hashmap_node = NULL;
hashmap->map_size = old_size * 2;
HashmapNode **new_node_list = calloc(hashmap->map_size, sizeof(HashmapNode*));
if (new_node_list != NULL)
{
for(i = 0; i map_size; i++)
{
HashmapNode new_nodep = NULL;
new_nodep = malloc(sizeof(HashmapNode));
if (new_nodep == NULL)
{
// **PUT ERROR HANDLING CODE HERE**
}
else
{
memset(*new_nodep, 0, sizeof(HashmapNode); // Initialize memory to null values
new_node_list[i] = new_nodep;
}
}
}
else
{
// **PUT ERROR HANDLING CODE HERE**
}In C Always Initialize the Variables
In C there is no guarantee that a variable on the stack is going to
be initialized to zero or NULL. To avoid unknown behavior and unknown results initialize all variables when they are declared. This has saved me a lot of debugging over the years. When I was writing code that integrated many libraries this was one of the type of bugs I often had to track down.
int i = 0;
int hash_index = 0;
int old_size = hashmap->map_size;
HashmapNode *hashmap_node = NULL;Portability
The HashMap.c file contains
#include "hashmap.h"but it is named HashMap.h in the question. Some operating systems ignore case in file names, others are quite strict. To avoid portability issues make sure the name of the included file is the same for both the operating system and in the #include.
calloc() versus malloc()
The function calloc(size_t n_items, size_t size_of_one_location) is meant to allocate arrays of some item. It makes the code clearer with similar results.
Code Snippets
HashmapNode *hashmap_node = NULL;
hashmap->map_size = old_size * 2;
HashmapNode **new_node_list = calloc(hashmap->map_size, sizeof(HashmapNode*));
if (new_node_list != NULL)
{
for(i = 0; i < hashmap->map_size; i++)
{
HashmapNode new_nodep = NULL;
new_nodep = malloc(sizeof(HashmapNode));
if (new_nodep == NULL)
{
// **PUT ERROR HANDLING CODE HERE**
}
else
{
memset(*new_nodep, 0, sizeof(HashmapNode); // Initialize memory to null values
new_node_list[i] = new_nodep;
}
}
}
else
{
// **PUT ERROR HANDLING CODE HERE**
}int i = 0;
int hash_index = 0;
int old_size = hashmap->map_size;
HashmapNode *hashmap_node = NULL;#include "hashmap.h"Context
StackExchange Code Review Q#141493, answer score: 6
Revisions (0)
No revisions yet.