Efficiency Test

Can anyone write this more efficient??

When the song is changed, the old track ## is recorded.
The idea is to then loop through the track list and find a track which actually has a path/filename associated with it.
Also, to make sure you stay within range of 0 and maxsongs.

The reason for this need, is flexibility and chance error removal. See, the music list is modable and each track has an id. Assuming someone lists song 0-20 buts skips 5-15 by leaving them blank I wish to avoid errors. Also if someone says go from track 50 and tries to go back 1000 songs...I want the code to derive the proper song, as shown in the RangeCheck.

I am just curious to see if I did this with a drop of efficiency...

Music_List is a vector of strings.

PHP Code:

---

BOOL SfxEngine::Play_FMOD()
{    
    short int DELIM = 1;
    if (m_OldFMOD > m_CurrentFMOD) DELIM = -1;
    else if (m_OldFMOD < m_CurrentFMOD) DELIM = 1;
    
    //stop all songs
    FMUSIC_StopAllSongs();
    
    //Out of range checks -- no song checks
    RangeCheck();
        
    while ( Music_List[m_CurrentFMOD].empty() )
    {
        m_CurrentFMOD += DELIM;
        RangeCheck();
    }

    m_tmpMusicHandle = FMUSIC_LoadSong(Music_List[m_CurrentFMOD].c_str() );
    if (!m_tmpMusicHandle)    return FALSE;

    FMUSIC_PlaySong(m_tmpMusicHandle);
    
    return TRUE;
}


void SfxEngine::RangeCheck()
{
    if    (m_CurrentFMOD < 0)            m_CurrentFMOD += m_MaxSongs + abs(m_CurrentFMOD);
    else if (m_CurrentFMOD > m_MaxSongs)    m_CurrentFMOD = 0;
} 


---

Once again you seem to be optimizing too early. Song changing is a very rare event, if it took a hundredth of a second still nobody would care.


I'd write this:

Code:

---

    short int DELIM = 1;
    if (m_OldFMOD > m_CurrentFMOD) DELIM = -1;
    else if (m_OldFMOD < m_CurrentFMOD) DELIM = 1;

---

As this:

Code:

---

short int DELIM = m_OldFMOD > m_CurrentFMOD ? -1 : 1;

---

This is a mere code size issue, though. The runtime saving is one assignment that most likely gets optimized away anyway.

If m_MaxSongs is really the song count, then your RangeCheck() function is broken. It also employs one more comparison than is necessary (you already know it's < 0, there's no need to call abs - in fact there's no need for the calculation at all.):

Code:

---

void SfxEngine::RangeCheck()
{
    if(m_CurrentFMOD < 0) {
        m_CurrentFMOD = m_MaxSongs - 1;
    } else if(m_CurrentFMOD > m_MaxSongs) {
        m_CurrentFMOD = 0;
    }
}

---

Finally, I'd put the LoadSong call into the loop. This checks not only if the entry is empty, but also if the file actually exists:

Code:

---

    m_tmpMusicHandle = 0;
    while (!m_tmpMusicHandle)
    {
        m_CurrentFMOD += DELIM;
        RangeCheck();
        if(!Music_List[m_CurrentFMOD].empty()) {
            m_tmpMusicHandle = FMUSIC_LoadSong(Music_List[m_CurrentFMOD].c_str() );
        }
    }

---