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


Vadim

Jean-Philippe Barrette-LaPierre wrote:

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.

I think everyone has a different way of perceiving what should be a simple
way of using lists, but everyone knows how to use std::list. I think that a
library should always expose the STL. It means that you can use a utility
class, but expose only the STL. After that, if a user wants to use a
convenient class, he could use one. But I think users will always use a
different utility class than you. So, instead of exposing your utility
class, you should expose the STL one.

Within OpenWengo it means that if we want to use the STL for libs/audio, we
just need to add a constructor with a std::list as an argument. Then, you
wouldn't need to change any code within openwengo, and the library wouldn't
be dependant of your utility class.

What do you think?

Here's a patch. You free to choose if you want to apply it.

------------------------------------------------------------------------

Index: libs/sound/include/AudioDevice.h
===================================================================
--- libs/sound/include/AudioDevice.h    (revision 3933)
+++ libs/sound/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: libs/sound/src/unix/AudioDevice.cpp
===================================================================
--- libs/sound/src/unix/AudioDevice.cpp (revision 3933)
+++ libs/sound/src/unix/AudioDevice.cpp (working copy)
@@ -25,11 +25,11 @@

#include <stdio.h>

-StringList AudioDevice::getInputMixerDeviceList() {
+std::list< std::string > AudioDevice::getInputMixerDeviceList() {
        int i, numDevices;
        const PaDeviceInfo *deviceInfo;
        PaError err;
-       StringList deviceList;
+       std::list< std::string >  deviceList;
        
        Pa_Initialize();
@@ -42,7 +42,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 +50,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,17 +61,17 @@
                        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;
-       StringList deviceList;
+       std::list< std::string > deviceList;
        
        Pa_Initialize();
@@ -84,7 +84,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 +92,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 +103,7 @@
                        deviceName += ": ";
#endif
                        deviceName += deviceInfo->name;
-                       deviceList += deviceName;
+                       deviceList.push_back(deviceName);
                }
        }
        return deviceList;
@@ -132,13 +132,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 "";
@@ -153,31 +153,35 @@
}

int AudioDevice::getWaveOutDeviceId(const std::string & deviceName) {
-       StringList deviceList = getOutputMixerDeviceList();
+       std::list< std::string > deviceList = getOutputMixerDeviceList();

        if( deviceName.length() ) {
- for(unsigned int currentDeviceIndex = 0; - currentDeviceIndex < deviceList.size(); - ++currentDeviceIndex) {
-                 if( deviceName == deviceList.get(currentDeviceIndex)) {
+ unsigned int currentDeviceIndex = 0; + for(std::list< std::string >::iterator it = deviceList.begin();
+                   it != deviceList.end();
+                   it++) {
+                 if( deviceName == *it ) {
                    return currentDeviceIndex;
                  }
+                 currentDeviceIndex++;
                }
        }
        return 0;
}

int AudioDevice::getWaveInDeviceId(const std::string & deviceName) {
-       StringList deviceList = getInputMixerDeviceList();
+       std::list< std::string > 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(std::list< std::string >::iterator it = deviceList.begin();
+                   it != deviceList.end();
+                   it++) {
+                 if( deviceName == *it ) {
+                   return currentDeviceIndex;
+                 }
+                 currentDeviceIndex++;
+               }
        }
        return 0;
}
Index: libs/util/include/List.h
===================================================================
--- libs/util/include/List.h    (revision 3933)
+++ libs/util/include/List.h    (working copy)
@@ -22,6 +22,7 @@

#include <exception/OutOfRangeException.h>

+#include <list>
#include <vector>

/**
@@ -38,6 +39,18 @@
        List() {
        }

+ List(const std::vector< T > &other) + : _list(other) {
+       }
+
+       List(const std::list< T > &other) {
+         for(typename std::list< T >::const_iterator it = other.begin();
+             it != other.end();
+             it++) {
+           _list.push_back(*it);
+         }
+       }
+       
        virtual ~List() {
                clear();
        }
------------------------------------------------------------------------

_______________________________________________
Wengophone-devel mailing list
[email protected]
http://dev.openwengo.com/mailman/listinfo/wengophone-devel


_______________________________________________
Wengophone-devel mailing list
[email protected]
http://dev.openwengo.com/mailman/listinfo/wengophone-devel

Reply via email to