On Tue, Sep 20, 2022 at 5:49 PM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > On 9/19/2022 6:02 PM, Junio C Hamano wrote: > > Derrick Stolee <derrickstolee@xxxxxxxxxx> writes: > > > >>> int32_t array_container_write(const array_container_t *container, char *buf); > >>> + > >>> +int array_container_network_write(const array_container_t *container, > >>> + int (*write_fn) (void *, const void *, size_t), > >>> + void *data); > >> > >> Should we make write_fn a defined type? I'm not sure I've seen this > >> implicit type within a function declaration before. > > > > Unless we can point out why having a named type is a good idea > > (e.g. we add such a function pointer as a member of a struct, or we > > keep a variable of that type somewhere), I actually would prefer to > > do without them. > > > > Perhaps there are some more important reasons I am missing why we > > often come up with explicit types for callback function pointers in > > many parts of our API, but if there aren't, my preference actually > > is to lose them, not add more of them. > > > > Hmph.... could "a typedef can become a place to give definitive > > documentation for the class of callback functions" be a good reason > > why we would want one? I dunno. > > > > In the posted patch, readers cannot tell what kind of three > > parameters they are supposed to give to write_fn(). > > This is exactly my reasoning. Having a clear definition gives us an > opportunity to document what each parameter is for, even if it is > just a variable name. Agreed. > This anonymous type is used in multiple places, so it can be helpful > to know that the type is connected across call sites or a stack of > method calls. > > In the unlikely event that we needed to modify this callback > signature, changing it in one place makes it clear that we cover > all connected uses instead of tracking all of these anonymous > functions across multiple methods. Got it. Thanks!
