Re: [PATCH 1/5] hashmap: add enum for hashmap free_entries option
- From
Heiko Voigt <hvoigt@hvoigt.net>
- Date
- Jun 10, 2014, 10:17 UTC
- Message-ID
- <20140610101744.GA23370@t2784.greatnet.de>
- In-Reply-To
- <5391FFC3.5010001@gmail.com>
On Fri, Jun 06, 2014 at 07:52:03PM +0200, Karsten Blees wrote:
Show 22 quoted lines
> Am 05.06.2014 08:06, schrieb Heiko Voigt:
> > This allows a reader to immediately know which options can be used and
> > what this parameter is about.
> >
> [...]
> > -void hashmap_free(struct hashmap *map, int free_entries)
> > +void hashmap_free(struct hashmap *map, enum hashmap_free_options free_entries)
> [...]
> >
> > +enum hashmap_free_options {
> > + HASHMAP_NO_FREE_ENTRIES = 0,
> > + HASHMAP_FREE_ENTRIES = 1,
> > +};
>
> This was meant as a boolean parameter. Would it make sense to have
>
> enum boolean {
> false,
> true
> };
>
> or similar in some central place?The intention of Jonathans critique here[1] was that you do not see what this parameter does on the callsite. I.e.:
hashmap_free(&map, 1);
compared to
hashmap_free(&map, HASHMAP_FREE_ENTRIES);
A boolean basically transfers the same information and would not help the reader here.
Cheers Heiko
[1] http://article.gmane.org/gmane.comp.version-control.git/243917