On Tue, 2013-08-06 at 08:05 +0300, Tanu Kaskinen wrote: > On Tue, 2013-08-06 at 09:28 +0530, Arun Raghavan wrote: > > 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. > > Do you remember the discussion about reporting errors with ints vs. > bools? My recollection is that we agreed to use ints. It's also written > on > http://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/CodingStyle/ Hmm, you're right. Let me push out an updated patch. -- Arun
