Re: [PATCH 1/5] hashmap: add enum for hashmap free_entries option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 06, 2014 at 07:52:03PM +0200, Karsten Blees wrote:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]