#Using struct Personally, I think you made the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

#Prefer const and functions to macros You've defined the array size using a macro. (削除) This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety. (削除ここまで) Edit: That's a C++-ism that doesn't work in C. My bad! But the rest of the advice in the next paragraph is correct as far as I know.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general, if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be (assuming you can use the constant in the desired way).

#Errors There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

#Naming A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.

Using struct

Personally, I think you made the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

Prefer const and functions to macros

You've defined the array size using a macro. (削除) This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety. (削除ここまで) Edit: That's a C++-ism that doesn't work in C. My bad! But the rest of the advice in the next paragraph is correct as far as I know.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general, if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be (assuming you can use the constant in the desired way).

Errors

There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

Naming

A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.

#Using struct Personally, I think you maid the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

#Prefer const and functions to macros You've defined the array size using a macro. (削除) This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety. (削除ここまで) Edit: That's a C++-ism that doesn't work in C. My bad! But the rest of the advice in the next paragraph is correct as far as I know.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be (assuming you can use the constant in the desired way).

#Errors There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

#Naming A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.

#Using struct Personally, I think you made the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

#Prefer const and functions to macros You've defined the array size using a macro. (削除) This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety. (削除ここまで) Edit: That's a C++-ism that doesn't work in C. My bad! But the rest of the advice in the next paragraph is correct as far as I know.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general, if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be (assuming you can use the constant in the desired way).

#Errors There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

#Naming A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.

#Using struct Personally, I think you maid the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

#Prefer const and functions to macros You've defined the array size using a macro. This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be.

#Errors There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

#Naming A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.

#Using struct Personally, I think you maid the right choice by using a struct with a char* and int in it rather than 2 arrays of char* and int, respectively. The Pair is a conceptual unit of data in your app, so it makes sense to put them together. If you had 2 separate arrays, it would be easy for them to get out of synch and hard to debug why that happened. Nice work!

#Prefer const and functions to macros You've defined the array size using a macro. (削除) This puts you at a disadvantage while programming. You've removed the type information for the value. By making it:

const int ARR_SIZE = 10;

you get type safety. (削除ここまで)Edit: That's a C++-ism that doesn't work in C. My bad! But the rest of the advice in the next paragraph is correct as far as I know.

With macros that take parameters you run the risk of them being used in unexpected ways and causing hard to debug issues. Luckily, you haven't done that here. But in general if you find yourself reaching for a macro, ask yourself if you would be better off with a constant or a function. You almost always will be (assuming you can use the constant in the desired way).

#Errors There are some errors in your code. In make_pair(), you don't check to see if malloc() succeeded. If it fails, no memory is allocated, and pair points to NULL. When you try to assign pair->val or pair->word you will crash.

If you did fix that, table_insert() uses the result of make_pair() without checking to see if it's NULL first. This won't crash immediately, because you're just assigning the array pointer in tbl->pairs[tbl->sz] to have the value of pair. What will happen is later when you try to search or print or insert another item, you'll get a crash when you iterate over that entry in the table and try to do anything with it.

You could make these errors not possible by not dynamically allocating the array entries. Simply make the array an array of Pair structs rather than a pointer to them.

#Naming A lot of your names are really good. Pair and Table are decent, readable names for this task. make_pair(), table_insert(), etc. are informative. Some could be improved, though. tbl does not save you much typing over table. Just use the whole word. Same with sz vs. size. i is acceptable as a loop variable name, but it would be even better if it was more descriptive. For example, entry_index or pair_index. It should describe what you're iterating over.