How did I do?

Started by
1 comment, last by Gor435 18 years, 8 months ago
I wrote my first FMOD program and I would like some feedback on how I did is the code readable, a mess, or just not worth anything. Please let me know what you think. main.cpp

//My attempt to use the FMOD Lib for a cd player

#include "main.h"

LRESULT CALLBACK WindowProcedure (HWND, UINT, WPARAM, LPARAM);

//Make the class name and the app title into global variables
const char szClassName[ ] = "Windows App";
const char title[] = "Gor435's CD Player";

char drive;
int track = 1;
int volume = 100;
int temp = 0;
HMENU menu;

int WINAPI WinMain (HINSTANCE hThisInstance,
                    HINSTANCE hPrevInstance,
                    LPSTR lpszArgument,
                    int nFunsterStil)

{
    //init FMOD
    if (!FSOUND_Init(32000, 2, FSOUND_INIT_USEDEFAULTMIDISYNTH))
    {
      //if FMOD fails dispaly message and exit  
      MessageBox(NULL, "FMod Failed", "Aww Crap", MB_ICONERROR | MB_OK);
      return 0;  
    }        
    
    HWND hwnd;               
    MSG messages;            
    WNDCLASSEX wincl;       

    /* The Window structure */
    wincl.hInstance = hThisInstance;
    wincl.lpszClassName = szClassName;
    wincl.lpfnWndProc = WindowProcedure;      //This function is called by windows
    wincl.style = CS_DBLCLKS;                 
    wincl.cbSize = sizeof (WNDCLASSEX);
    wincl.hIcon = (HICON)LoadImage (NULL, ICON, IMAGE_ICON, 32, 32, LR_LOADFROMFILE);
    wincl.hIconSm = (HICON)LoadImage (NULL, SMICON, IMAGE_ICON, 16, 16, LR_LOADFROMFILE);
    wincl.hCursor = LoadCursor (NULL, IDC_ARROW);
    wincl.lpszMenuName = NULL;                 
    wincl.cbClsExtra = 0;                      
    wincl.cbWndExtra = 0;                      
    wincl.hbrBackground = (HBRUSH) COLOR_BACKGROUND;

    //Register the window class, and if it fails quit the program
    if (!RegisterClassEx (&wincl))
        return 0;

    //The class is registered, let's create the window
    hwnd = CreateWindowEx (
           0,                            //Extended possibilites for variation 
           szClassName,                  //Classname 
           title,                        //Title Text 
           WS_CAPTION | WS_POPUPWINDOW,  //default window 
           0,                            //top left corner of the screen 
           0,                            //top left corner of the screen 
           175,                          //The programs width 
           52,                           //and height in pixels 
           HWND_DESKTOP,                 //The window is a child-window to desktop 
           NULL,                         //No menu 
           hThisInstance,                //Program Instance handler 
           NULL                          //No Window Creation data 
           );

    //Make the window visible on the screen
    ShowWindow (hwnd, nFunsterStil);
    
    //Prepare and setup menu
    menu = LoadMenu(hThisInstance, MAKEINTRESOURCE(ID_MENU));
    SetMenu(hwnd, menu);
    

    //Run the message loop. It will run until GetMessage() returns 0
    while (GetMessage (&messages, NULL, 0, 0))
    {
        //Translate virtual-key messages into character messages
        TranslateMessage(&messages);
        //Send message to WindowProcedure
        DispatchMessage(&messages);
    }

    
    return messages.wParam;
}



LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{
    MENUITEMINFO info;
        
    switch (message)                  //handle the messages
    {
        case WM_DESTROY:
            
            PostQuitMessage(0);       //send a WM_QUIT to the message queue
            break;
        case WM_COMMAND:
            {
                switch(wParam)
                {
                    case FOPEN://open the cd tray
                        {
                            FSOUND_CD_OpenTray(0, 1); //open the tray
                            break;
                        }
                    case FCLOSE://close the cd tray
                        {
                            FSOUND_CD_OpenTray(0, 0); //close the tray
                            break;
                        }    
                    case FEXIT://close the program
                        {
                            FSOUND_CD_Stop(0); //stop playing the cd
                            FSOUND_Close(); //shut down Fmod
                            DestroyMenu(menu); //get rid of menu so it doesn't stay in memory 
                            PostQuitMessage(0);  /* send a WM_QUIT to the message queue */
                            break;
                        }    
                    case CPLAY://play
                        {
                            FSOUND_CD_SetVolume(0, volume); // set the volume
                             
                            //set the play mode to play all the way through the cd
                            FSOUND_CD_SetPlayMode( 0, FSOUND_CD_PLAYCONTINUOUS); 
                            FSOUND_CD_Play( 0, track); //play the selected track on the cd in the default cd drive
                            break;
                        }    
                    case CPAUSE://pause
                        {  
                                                                                                            
                            if (!FSOUND_CD_GetPaused(0))// if the cd is not paused
                            {
                                FSOUND_CD_SetPaused(0, 1);//pause the cd
                            }
                            else
                            { 
                                FSOUND_CD_SetPaused(0, 0);//unpause the cd
                            }        
                            
                            
                            break;
                        }    
                    case CSTOP://Stop
                        {
                            FSOUND_CD_Stop(0);//stop the cd
                            break;
                        }    
                    case CREW://rewind type function
                        {
                            MessageBox(NULL, "Funtion Not Yet Implemented", "Aww Crap", MB_ICONERROR | MB_OK);
                            break;
                        }    
                    case CFF://fast forward type funtion
                        {
                            MessageBox(NULL, "Funtion Not Yet Implemented", "Aww Crap", MB_ICONERROR | MB_OK);
                            break;
                        }
                    case CNT://next track
                        {
                            int newtrack = 0;
                            newtrack = FSOUND_CD_GetNumTracks(0);
                            track++;
                            
                            if (track > newtrack)
                            track = 1;
                            
                            FSOUND_CD_Play( 0, track);
                            break;
                        }    
                    case CBT:// last track or back track
                        {
                            int newtrack = 0;
                            newtrack = FSOUND_CD_GetNumTracks(0);
                            
                            track--;
                            
                            if (track == 0)
                            track = newtrack;
                            
                            FSOUND_CD_Play( 0, track);
                            break;
                        }
                    case CFT://First track
                        {
                            track = 1;
                            FSOUND_CD_Play( 0, track);
                            break;
                        }
                    case VUP://volume up
                        {
                            volume += 50;
                            if (volume >= 250)
                            volume = 250;
                            
                            FSOUND_CD_SetVolume(0, volume);
                            break;
                        }
                    case VDOWN://volume down
                        {
                            volume -= 50;
                            if (volume <= 0)
                            volume = 0;
                            
                            FSOUND_CD_SetVolume(0, volume);
                            break;
                        }
                    case VMUTE://Mute the cd
                        {
                            if (volume == 0)
                            {
                            volume = temp;
                            }    
                            else
                            {
                            temp = volume;
                            volume = 0;
                            }
                                
                            FSOUND_CD_SetVolume(0, volume);
                            break;
                        }                              
                    case HHELP://help
                        {
                            MessageBox(NULL, "No help file", "Aww Crap", MB_ICONERROR | MB_OK);
                            break;
                        }    
                    case HABOUT://about
                        {
                            // :)
                            MessageBox(NULL, "Programmed by Shawn McBroom Also Known As Gor435", "About", MB_ICONINFORMATION | MB_OK);
                            break;
                        }    

                }    
            }    
        default:                      //for messages that we don't deal with
            return DefWindowProc (hwnd, message, wParam, lParam);
    }

    DestroyMenu(menu);//get rid if the menu so it doesn't stay in memory
    FSOUND_Close();//Shut down FMOD

    return 0;
}

main.h

#include <windows.h>
#include "fmod.h"
#include "fmod_errors.h"

#define WIN32_LEAN_AND_MEAN
#define WIN32_EXTRA_LEAN

#ifndef main_h
#define main_h
#define ICON "player icon.ico"
#define SMICON "cd.ico"

#define ID_MENU 500

#define FCLOSE  198
#define FOPEN   199
#define FEXIT   200
#define CPLAY   201
#define CPAUSE  202
#define CSTOP   203
#define CREW    204
#define CFF     205
#define HHELP   206
#define HABOUT  207
#define CNT     208
#define CBT     209
#define CFT     210
#define VUP     211
#define VDOWN   212
#define VMUTE   213

#endif

resources.rc

100 ICON MOVEABLE PURE LOADONCALL DISCARDABLE "CD Player.ico"

#include <windows.h>
#include "main.h"

500 MENU
BEGIN
    POPUP "&File"
    BEGIN
        MENUITEM "&Open Tray", FOPEN
        MENUITEM "&Close Tray", FCLOSE
        MENUITEM SEPARATOR
        MENUITEM "&Exit", FEXIT
    END
    
    POPUP "&Controls"
    BEGIN
        MENUITEM "&Play", CPLAY
        MENUITEM "P&ause", CPAUSE
        MENUITEM "&Stop", CSTOP
        MENUITEM SEPARATOR
        MENUITEM "&Next Track", CNT
        MENUITEM "&Back Track", CBT
        MENUITEM "F&irst Track", CFT
    END
    
    POPUP "&Volume"
    BEGIN
    MENUITEM "Volume &Up", VUP
        MENUITEM "Volume &Down", VDOWN
        MENUITEM "&Mute", VMUTE
    END
    
    POPUP "&Help"
    BEGIN
        MENUITEM "He&lp", HHELP
        MENUITEM "&About", HABOUT
    END
END


Gor435 - My Journal - MySpace - Facebook
Advertisement
Definately a hackey way of doing it. With a little work, you could have it cleaned up, and running better. Overall, it's not too bad, but there are a few problems with it.

You forgot to shut down FMod when the window was closed, and if the window class registration failed. Also, I don't think I saw 'MENUITEMINFO info;' (in 'WindowProcedure') being used anywhere, although I could have missed it.

As for the global variables, you don't need 'szClassName', 'title', and 'menu' to be global (your current implementation requires 'menu' to be global, but it doesn't have to be).

Consider renaming the 'temp' variable too, since it's very vague (I had to go searching to find out it's where you store the old volume before setting the volume to zero to mute it).

Also, avoid the copy and paste jobs wherever possible. It's fairly obvious, since your first 2 variables 'szClassName' and 'title' show it, as well as the 2 different commenting styles (C's /* */ version, and the C++ // version). It's not necessary you stick to the standards, but keep it consistant.

Finally, incase you didn't know, the only reason the braces are used inside switch statements is because variables can't be created there. In other words, you don't need so many of them. You can even remove some of them all together, for instance, you can go to the next track like this:

case CNT:  track++;  if (track > FSOUND_CD_GetNumTracks(0)){    track = 1;  }  FSOUND_CD_Play(0,track);  break;
Quote:Original post by Gorax
Definately a hackey way of doing it. With a little work, you could have it cleaned up, and running better. Overall, it's not too bad, but there are a few problems with it.


You forgot to shut down FMod when the window was closed, and if the window class registration failed. Also, I don't think I saw 'MENUITEMINFO info;' (in 'WindowProcedure') being used anywhere, although I could have missed it.


No MENUITEMINFO info; and yeah sorry missed that in the class registration.

Quote:
As for the global variables, you don't need 'szClassName', 'title', and 'menu' to be global (your current implementation requires 'menu' to be global, but it doesn't have to be).

Consider renaming the 'temp' variable too, since it's very vague (I had to go searching to find out it's where you store the old volume before setting the volume to zero to mute it).

Also, avoid the copy and paste jobs wherever possible. It's fairly obvious, since your first 2 variables 'szClassName' and 'title' show it, as well as the 2 different commenting styles (C's /* */ version, and the C++ // version). It's not necessary you stick to the standards, but keep it consistant.


I think you misunderstand there is no cut and paste job here it is a Dev-C++
windows gui template. I simply added my code and modified some of the comments.

Quote:
Finally, incase you didn't know, the only reason the braces are used inside switch statements is because variables can't be created there. In other words, you don't need so many of them. You can even remove some of them all together, for instance, you can go to the next track like this:


yeah I knew that. I use braces for my readability.

Thank you for your comments.
Gor435 - My Journal - MySpace - Facebook

This topic is closed to new replies.

Advertisement