On Mon, 2013-08-05 at 09:24 +0300, Tanu Kaskinen wrote: > 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 Yup, that was my primary reason. > 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? Sure, it's en route. Used a bool, though. -- Arun
