search

eteran / c-vector

A dynamic array implementation in C similar to the one found in standard C++
MIT License
737 stars 109 forks source link

Oddly named macro #47

Closed MACAYCZ closed 8 months ago

MACAYCZ commented 1 year ago

I think macro named cvector_free_each_and_free defined in cvector_utils.h should be named cvector_for_each_and_free

eteran commented 1 year ago

I agree that it's a bit awkwardly named... but I'm not sure your proposed name is any better. What the macro does is free each element, and then frees the vector. So that's what it's currently called. "for_each_and_free" does not really express what it does very well IMO.

I'm open to a better name though.

eteran commented 1 year ago

Upon re-visiting the code, I think you may be right. It was originally written as a free all macro, but it clearly just calls an arbitrary function on each element before freeing.

Honestly, I think the macro may just be better off being removed since there is very little use in calling a function on each element and then freeing... unless you're freeing each element, which the cvector_free macro already does.

I'll think on it.

RobLoach commented 1 year ago

I have a look at this, and my expectation would be that this combination of functions would satisfy what this macro does. As seen at the bottom of test.c...

// cvector_free_each_and_free(str_vect, free);
cvector_set_elem_destructor(str_vect, free);
cvector_free(str_vect);

Unfortunately, when we do that, I get...

free(): invalid pointer
eteran commented 8 months ago

I have removed the macro since it was poorly named and superceded by the cvector_set_elem_destructor feature. Tests have been updated accordingly and pass.

Thanks for the patience and thoughtful discussion!