On Mon, 2013-08-05 at 11:44 +0530, Arun Raghavan wrote: > On Mon, 2013-08-05 at 09:03 +0300, Tanu Kaskinen wrote: > > On Sat, 2013-08-03 at 11:41 +0530, Arun Raghavan wrote: > > > This makes it easier for users of this API to add/updated a volume > > > factor by doing a _remove_volume_factor() followed by an > > > add_volume_factor(), rather than having to either remember whether this > > > is the first set operation or have an API to query whether a factor has > > > already been set. > > > > There is a query API already: > > pa_hashmap_contains(i->volume_factor_items, key) > > > > Oops, I was wrong, that function doesn't exist (it probably should be > > added). The same thing can be achieved with pa_hashmap_get() too, > > though. > > I think it's a bit ugly to expect users of that API to check the > underlying structure itself first, though. Yes, but only slightly ugly. We generally allow reading object variables directly across classes (and to some extent write, but in my opinion that shouldn't be allowed). Anyway, I changed my mind about the change even before your reply. The reason is that I think it's ugly to require callers to always check the object state before calling a function. The function should fail if the state is not good (fail in some other way than crashing, that is). I'd like the failure to be reflected in the return value, so could you change the void function to return an int? -- Tanu
