I'm trying to implement a hash table in C so the idea is simple:
1- the user enters a string 2- the program adds it to the table 3- prints the whole table
but I noticed that the program replaces all previous values with the new one !! THE CODE:
//edit
//initialize array to empty string
void init(char* table[]){
for (int i=0; i<T_LEN; i ){
table[i] = "";
}
}
int hash(char* str){
int result = 0;
for (int i=0; i<sizeof(str); i ){
result = str[i];
}
return result % T_LEN;
}
void add(char* table[]){
char* input;
printf("> ");
scanf("%s", (char*)&input);
int index = hash((char*)&input);
table[index] = (char*)&input;
return;
}
int main(){
char* hash_table[T_LEN];
init(hash_table);
while(1){
add(hash_table);
print_table(hash_table);
}
return 0;
}
RESULT:
> abc
1 -------
2 -------
3 -------
4 -------
5 abc
6 -------
7 -------
8 -------
9 -------
10 -------
> esd
1 -------
2 -------
3 -------
4 -------
5 esd
6 -------
7 esd
8 -------
9 -------
10 -------
> ee
1 -------
2 -------
3 ee
4 -------
5 ee
6 -------
7 ee
8 -------
9 -------
10 -------
EDIT: the site says "It looks like your post is mostly code; please add some more details." so here I'm :D
CodePudding user response:
some problems with your code:
return;
at the end of the function calledadd
is redundant which means that it has no meaning as in all the cases the function will end at this point
the compiler is giving the warning
while(1)
in themain
as you don't return from the function, so you can doint __attribute__((noreturn)) main()
to tell the compiler that the main
will never return
in the function called
init
, as you are declaring an array of pointers, then it's better to write:table[i] = malloc(sizeof(char) * MAX_NAME_LEN); table[i][0] = '\0';
instead of:
table[i] = "";
to reserve memory in heap for the string not in the read-only memory.
in the lines:
scanf("%s", (char*)&input); int index = hash((char*)&input);
the pointer called input
is already of type char*
, so you don't have to cast it as it's not a better practice to cast everything, also you should reserve a space for the input
variable in the heap memory like:
char* input = malloc(sizeof(char) * MAX_NAME_LEN);
and so your code becomes:
char* input = malloc(sizeof(char) * MAX_NAME_LEN);
printf("> ");
scanf("%s", input);
int index = hash(input);
table[index] = input;
instead of
sizeof()
in the line:for (int i=0; i<sizeof(str); i )
I think you should use strlen
instead like:
for (int i=0; i<strlen(str); i )
as sizeof(str) = 4
as sizeof(pointer) = 4
as the pointers has fixed size
with all this being said, this is the edited code:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#define T_LEN 10
#define MAX_NAME_LEN 20
void print_table(char* table[])
{
for (int i = 0; i < T_LEN; i) {
printf("%d\t\t%s\n",i, table[i]);
}
}
//edit
//initialize array to empty string
void init(char* table[]){
for (int i=0; i<T_LEN; i ){
table[i] = malloc(sizeof(char) * MAX_NAME_LEN);
table[i][0] = '\0';
}
}
int hash(char* str){
int result = 0;
for (int i=0; i<strlen(str); i ){
result = str[i];
}
return result % T_LEN;
}
void add(char* table[]){
char* input = malloc(sizeof(char) * MAX_NAME_LEN);
printf("> ");
scanf("%s", input);
int index = hash(input);
table[index] = input;
}
int __attribute__((noreturn)) main(){
char* hash_table[T_LEN];
init(hash_table);
while(1){
add(hash_table);
print_table(hash_table);
}
}
and this is some output:
>abc
0
1
2
3
4 abc
5
6
7
8
9
>esd
0
1
2
3
4 abc
5
6 esd
7
8
9
>ee
0
1
2 ee
3
4 abc
5
6 esd
7
8
9
>