> Jean-Philippe Barrette-LaPierre a écrit :
> >Le Janvier 17, 2006 11:55 AM, Jean-Philippe Barrette-LaPierre a écrit :
> >>I looked into libs/sound library. I think I'll use it into SFLPhone, but
> >>something bothers me. I see that you use a Utility class as a list. I
> >> don't have any problems to use that kind of utility classes within an
> >>application, because I admit that STL is not always simple to use. The
> >>problem here is that I'm not sure that this utility classes should be
> >>exposed within libs/sound.
>
> Is the problem the fact that in order to use this library in another
> app, you would have to learn the API provided by return values types (in
> this case, StringList)?
Exactly.
<snip>
> Since StringList and String are subclasses of STL classes, we could
> consider that the user of the API should know that she can use the STL
> API with the returned values.
> The user of this library alreay has to read the minimal documentation to
> know the API, the fact that some returned values' types are subclasses
> of STL types could be documented in the same place as well.
>
> What do you think about this?
>
The good thing with a simple library, as libs/sound, is that you can grasp the
"essence" of the library in a few minutes. I must admit that StringList is
very explicit. Even if you don't know the interface of StringList, you know
what is the purpose of this class. It doesn't "obscurify" the signature of a
function using it. I mean that function signatures like this:
std::list< std::string > getAudioDevices;
or like this
StringList getAudioDevices;
are very simple signature, and they are equally explicit of their purpose. But
imagine that you have a program that have 4 or 5 dependencies. You might end
trying to handle 4 or 5 different utility classes that deserves the same
purpose, but in different ways. STL was created to provide a standard way of
handling list, because it was a pain in the ... to handle all the utility
classes given by each used library.
BTW, I must agree with Vadim Lebedev when he says:
>I would suggest to do a local
>typedef std::list< std::string > StringList;
>inside the class AudioDevice.... The patch will become match smaller
>and the code more readable... IMO
I attached a modified version of the patch including Vadim suggestion.
P.S: The patch only include the unix version of the implementation, but I'll
provide a patch for other systems when we'll all agree on the way of handling
this "problem".
--
Jean-Philippe Barrette-LaPierre
Coder in wonderland
(try lisp and you'll see the light)
Index: include/AudioDevice.h
===================================================================
--- include/AudioDevice.h (revision 3933)
+++ include/AudioDevice.h (working copy)
@@ -21,7 +21,8 @@
#define AUDIODEVICE_H
#include <NonCopyable.h>
-#include <StringList.h>
+#include <list>
+#include <string>
/**
* Sound managing: get and set the wave out/in and default devices for Windows.
@@ -39,20 +40,20 @@
*
* @return list of input mixer devices
*/
- static StringList getInputMixerDeviceList();
+ static std::list< std::string > getInputMixerDeviceList();
/**
* Gets the list of output mixer devices.
*
* @return list of output mixer devices
*/
- static StringList getOutputMixerDeviceList();
+ static std::list< std::string > getOutputMixerDeviceList();
/**
* Gets the default playback device.
*
* @return the default playback device or an empty string if could not be found
- */
+a */
static std::string getDefaultPlaybackDevice();
/**
Index: src/unix/AudioDevice.cpp
===================================================================
--- src/unix/AudioDevice.cpp (revision 3933)
+++ src/unix/AudioDevice.cpp (working copy)
@@ -25,11 +25,13 @@
#include <stdio.h>
-StringList AudioDevice::getInputMixerDeviceList() {
+typedef std::list< std::string > StringList;
+
+std::list< std::string > AudioDevice::getInputMixerDeviceList() {
int i, numDevices;
const PaDeviceInfo *deviceInfo;
PaError err;
- StringList deviceList;
+ StringList deviceList;
Pa_Initialize();
@@ -42,7 +44,7 @@
fprintf( stderr, "An error occured while using the portaudio stream\n" );
fprintf( stderr, "Error number: %d\n", err );
fprintf( stderr, "Error message: %s\n", Pa_GetErrorText( err ) );
- return StringList();
+ return deviceList;
}
//iterate over devices
@@ -50,7 +52,7 @@
deviceInfo = Pa_GetDeviceInfo( i );
if( deviceInfo->maxInputChannels > 0 ) {
- String deviceName;
+ std::string deviceName;
if( i == Pa_GetHostApiInfo( deviceInfo->hostApi )->defaultInputDevice ) {
deviceName = "(default) ";
} else {
@@ -61,13 +63,13 @@
deviceName += ": ";
#endif
deviceName += deviceInfo->name;
- deviceList += deviceName;
+ deviceList.push_back(deviceName);
}
}
return deviceList;
}
-StringList AudioDevice::getOutputMixerDeviceList() {
+std::list< std::string > AudioDevice::getOutputMixerDeviceList() {
int i, numDevices;
const PaDeviceInfo *deviceInfo;
PaError err;
@@ -84,7 +86,7 @@
fprintf( stderr, "An error occured while using the portaudio stream\n" );
fprintf( stderr, "Error number: %d\n", err );
fprintf( stderr, "Error message: %s\n", Pa_GetErrorText( err ) );
- return StringList();
+ return deviceList;
}
//iterate over devices
@@ -92,7 +94,7 @@
deviceInfo = Pa_GetDeviceInfo( i );
if( deviceInfo->maxOutputChannels > 0 ) {
- String deviceName;
+ std::string deviceName;
if( i == Pa_GetHostApiInfo( deviceInfo->hostApi )->defaultOutputDevice ) {
deviceName = "(default) ";
} else {
@@ -103,7 +105,7 @@
deviceName += ": ";
#endif
deviceName += deviceInfo->name;
- deviceList += deviceName;
+ deviceList.push_back(deviceName);
}
}
return deviceList;
@@ -132,13 +134,13 @@
for( i=0; i<numDevices; i++ ) {
deviceInfo = Pa_GetDeviceInfo( i );
if( i == Pa_GetDefaultOutputDevice() ) {
- String deviceName = "";
+ std::string deviceName = "";
#ifdef PRINT_HOSTAPI
deviceName += Pa_GetHostApiInfo(deviceInfo->hostApi)->name;
deviceName += ": ";
#endif
deviceName += deviceInfo->name;
- return String(deviceName);
+ return deviceName;
}
}
return "";
@@ -156,12 +158,14 @@
StringList deviceList = getOutputMixerDeviceList();
if( deviceName.length() ) {
- for(unsigned int currentDeviceIndex = 0;
- currentDeviceIndex < deviceList.size();
- ++currentDeviceIndex) {
- if( deviceName == deviceList.get(currentDeviceIndex)) {
+ unsigned int currentDeviceIndex = 0;
+ for(StringList::iterator it = deviceList.begin();
+ it != deviceList.end();
+ it++) {
+ if( deviceName == *it ) {
return currentDeviceIndex;
}
+ currentDeviceIndex++;
}
}
return 0;
@@ -171,13 +175,15 @@
StringList deviceList = getInputMixerDeviceList();
if( deviceName.length() ) {
- for(unsigned int currentDeviceIndex = 0;
- currentDeviceIndex < deviceList.size();
- ++currentDeviceIndex) {
- if( deviceName == deviceList.get(currentDeviceIndex)) {
- return currentDeviceIndex;
- }
- }
+ unsigned int currentDeviceIndex = 0;
+ for(StringList::iterator it = deviceList.begin();
+ it != deviceList.end();
+ it++) {
+ if( deviceName == *it ) {
+ return currentDeviceIndex;
+ }
+ currentDeviceIndex++;
+ }
}
return 0;
}
_______________________________________________
Wengophone-devel mailing list
[email protected]
http://dev.openwengo.com/mailman/listinfo/wengophone-devel
