This code is very easy to read, and very straightforward in its implementation. Nice work! Overall, it looks really good. I would say that it does follow abstraction and encapsulation fairly well. Below are some things that I think could be improved.
Functions Should Do 1 Thing
#Functions Should Do 1 Thing
YourYour createList()
function does 3 possible things:
- Creates a new empty list with a set capacity
- Creates a new list with double the capacity of an existing list, then copies the existing list to it
- Creates a new list with half the capacity of an existing list, then copies the existing list to it
That's a little confusing. Also the name flag
for the second parameter is not very helpful. If I were to keep the function as-is, I'd rename it to operation
, or something like that to make it more clear. Usually a flag is a Boolean value of some sort. (It may be an int
but is usually on or off.)
So I'd break createList()
into 2 or 3 functions like this:
List* createList();
size_t setListSize(List* ptr, const size_t newSize);
Or you could break the second one into 2 functions, like increaseSize()
and decreaseSize()
, but I don't think that's necessary.
The new createList()
function would just do what's inside of the if (flag == NEW_LIST)
portion of your original function. For setListSize()
, I'd pass in the new desired size. You can check inside the function if it's greater or less than the current size and do the appropriate thing. And the return value should be the size it was set to, which leads to my next point…
Handle Errors
#Handle Errors
NoneNone of your functions handle error cases. What if you attempt to increase the size of the list and you're out of memory? What if you want to reduce the size of the list, but can't allocate the smaller-sized array before doing the copy? With your current createList()
function, it will return NULL
, but none of the callers check that return value, so they could crash. Even worse, you allocate a List
on the first line, then assume it worked and allocate the array. Then you assume that worked and start setting or copying to it. If any one of those steps fails, the next one will probably crash it.
If you return the size from setListSize()
, the caller can check to make sure it worked and react appropriately if it didn't.
Modify the Existing List
#Modify the Existing List
SinceSince your InsertItem()
and DeleteItem()
functions modify your list, the setListSize()
function should too. Rather than create a new list, it would create the new array (either larger or smaller than the current one), do the copy, then free the existing one and replace it with the new one.