[pulseaudio-discuss] [Vala] Issues will vala and pulse vapi
astavale at yahoo.co.uk
Sun Nov 22 10:51:31 PST 2015
> From: Aaron Paden <aaronbpaden at gmail.com>
> Sent: Sunday, 22 November 2015, 2:48
> Subject: Re: [Vala] Issues will vala and pulse vapi
>> From an object oriented design point of view your SinkSoureList
>> class is doing too much. Initialisation should be done elsewhere
>> and I think it is unusual to have GLib.MainLoop within a class.
>> Some refactoring would be helpful here to see the problem more
> Well I've taken a few classes but I'm definitely at the level of a
> hobbyist, so I have lots of gaps in my understanding of best
> practices. SinkSourceList's purpose is simply to hold a list of pulse
> sources/sinks regardless of what the rest of the application is or
> isn't doing.
Sounds very reasonable, although I wonder if SinkList and SourceList
should be separate classes.
The class you are creating is a wrapper around the asynchronous
PulseAudio API that blocks until the complete list is returned
or a timeout occurs. It's essentially a snapshot that you can
then iterate over later. I hope that's a fair summary.
You could think about using a Vala signal instead. Other objects
can hook into this to have a function called when the list is
ready. This is the Vala implementation of the observer pattern.
> It allows you to pass a MainContext to the constructor,> an only creates a MainLoop if MainContext is null. For example, you
> might use SinkSourceList to write a one off utility like pactl list
> short sources. In that case, the application logic really doesn't need
> a mainloop, and the mainloop becomes an implementation detail of pulse
> that can be abstracted away.
>From what I can see creating the list is dependent on a PulseAudio.Context.
I think it would be better to pass an instance of that in the constructor.
Then your initialize_sources method goes, but the code within it becomes
the constructor code block.
You can then create a factory function that returns a PulseAudio context
created in a way that fits your rules. So your current constructor and
initialize_pulse method are taken out of the current class completely
to create a separate factory function. This way you can re-use the context
in other objects you create later, instead of duplicating that code for
I hope that makes sense of what I meant when I said your current
class does too much.
> That was my reasoning, anyway. Could be completely faulty, and I'd
> love to be corrected if I've obviously doing something incorrect.
Obviously there isn't a single right way, but generally I find the
SOLID principles helpful when reviewing code. In this case the 'S'
of SOLID applies - the Single Responsibility Principle. You want
your object to create the list, not also set up the PulseAudio context.
All the best with it,
More information about the pulseaudio-discuss