Thread: how to optimize "for" statement

I have this code and I wont to optimize them:

here is a scrolling menu with max 15 line and loop must run for all station from database ( num_stations = number of stations from data base ).

Code:

#define MENU_SIZE 15

        int i;
        char text[ 32 ];
        const int num_stations = station_get_num_stations( cmd->station );
    osd_list_set_lines( cmd->osd, num_stations );

    osd_list_set_text( cmd->osd, 0, _("Station List") );


        for( i = 0; i < MENU_SIZE; i++ ) {
                if( cmd->curmenupos < MENU_SIZE ) {
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s",  station_get_channel( cmd->station, i ), station_get_name(  cmd->station, i ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE ) { 
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s",  station_get_channel( cmd->station, i + 1 ), station_get_name(  cmd->station, i + 1 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE + 1 ) {  
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s",  station_get_channel( cmd->station, i + 2 ), station_get_name(  cmd->station, i + 2 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE + 2 ) { 
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s",  station_get_channel( cmd->station, i + 3 ), station_get_name(  cmd->station, i + 3 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );

                ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
                /* n = num_stations */
                } else if ( cmd->curmenupos == MENU_SIZE + n ) {  
                   /* Station number + name */                    
                    snprintf( text, sizeof (text), "[%s] %s",  station_get_channel( cmd->station, i + n + 1 ), station_get_name(  cmd->station, i + n + 1 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                }
                cmd->curmenusize = MENU_SIZE + 1

my attempt:

Code:

        #define MENU_SIZE 15

        int i, j;
        char text[ 32 ];
        const int num_stations = station_get_num_stations( cmd->station );
        osd_list_set_lines( cmd->osd, num_stations );
        osd_list_set_text( cmd->osd, 0, _("Station List") );

        for( i = 0; i < MENU_SIZE; i++ ) {
            for( j = 0; j < num_stations - MENU_SIZE; j++ ) {
                if( cmd->curmenupos < MENU_SIZE ) {
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i ), station_get_name( cmd->station, i ) );
                   osd_list_set_text( cmd->osd, i + 1, text );
                } else if( cmd->curmenupos == MENU_SIZE + j ) {
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i + j + 1 ), station_get_name( cmd->station, i + j + 1 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                }
            }
        }
        cmd->curmenusize = MENU_SIZE + 1;

Thanks

How about sharing with us what is the purpose of this code?

//yes, I know that I can read and try to find out..., but...

Originally Posted by std10093

How about sharing with us what is the purpose of this code?

//yes, I know that I can read and try to find out..., but...

I try to optimize from this:

Code:

#define MENU_SIZE 15

        int i;
        char text[ 32 ];
        const int num_stations = station_get_num_stations( cmd->station );

        for( i = 0; i < MENU_SIZE; i++ ) {
                if( cmd->curmenupos < MENU_SIZE ) {
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i ), station_get_name( cmd->station, i ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE ) { 
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i + 1 ), station_get_name( cmd->station, i + 1 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE + 1 ) {  
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i + 2 ), station_get_name( cmd->station, i + 2 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                } else if ( cmd->curmenupos == MENU_SIZE + 2 ) { 
                    /* Station number + name */
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i + 3 ), station_get_name( cmd->station, i + 3 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );

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

                } else if ( cmd->curmenupos == MENU_SIZE + n ) {  
                   /* Station number + name */                    
                    snprintf( text, sizeof (text), "[%s] %s", station_get_channel( cmd->station, i + n + 1 ), station_get_name( cmd->station, i + n + 1 ) );
                    osd_list_set_text( cmd->osd, i + 1, text );
                }

               /* n = num_stations */

A loop executing 15 times is indeed a heavy loop needing optimisation.
But assuming optimisaton for size, what about removing this loop entirely? You would save up to several hundred bytes in the code section.

Originally Posted by kmdv

A loop executing 15 times is indeed a heavy loop needing optimisation.
But assuming optimisaton for size, what about removing this loop entirely? You would save up to several hundred bytes in the code section.

Sorry, in my first post is rong code:

#define NUM_STATIONS 15
must be

#define MENU_SIZE 15

the loop must run for all number of stations from database: num_stations = stations in data base, menu_size = scrooling menu with max 15 line

It doesn't change anything...

Are you really sure what you want to optimise? Does this loop take ages to finish its job?

Your loop seems to make no harm itself.

perhaps a more elegant solution is what I'm looking for.

This solution have a problem:

Example:

Number of stations in data base = 75

1 If cursor in in first line of MENU and I wont to move up, rather than jumping to line 75, cursor jump to line 61. ( max stations - 15)
2. If I scroll down all is fine: when cursor get line 75 and move down one line, jump to line 1 in MENU what is right.

By saying "optimise" you make other think about code optimisation (e.g., for speed), which is something completely different.
It is impossible to guess what you want to do by looking at a piece of broken and incomplete code. You might want to use an array to store some information, and instead of if-else statements, access that array at a specified index (index = cmd->curmenupos).

"Optimise for code size" is the idea here I believe.

What is wrong with the attempt you posted already?

Obvious (to me at least) question is the value of "cmd->curmenupos" changing inside the loop?

If not changing, I see no need for the "else if" statements being used. My hint only applies if value is not changing inside loop.

Note: I am guessing this is a homework assignment; therefore I am not giving you anymore help on it.

Hint: The answer is more a math answer than a programming logic answer.

Tim S.

Originally Posted by stahta01

Obvious (to me at least) question is the value of "cmd->curmenupos" changing inside the loop?

If not changing, I see no need for the "else if" statements being used. My hint only applies if value is not changing inside loop.

Note: I am guessing this is a homework assignment; therefore I am not giving you anymore help on it.

Hint: The answer is more a math answer than a programming logic answer.

Tim S.

Hello,
Yes, "cmd->curmenupos" changing inside the loop

MOVE UP:

Code:

cmd->curmenupos = (cmd->curmenupos + cmd->curmenusize - 1) % (cmd->curmenusize);

MOVE DOWN:

Code:

cmd->curmenupos = (cmd->curmenupos + 1) % (cmd->curmenusize);

This command ( cmd->curmenupos ) move up or down with one line in osd menu.

My problem is:
If cursor is in first line of OSD MENU and I want to move up, rather than jumping to line 75, cursor jump to line 61.

See screenshot:


If I scroll down all is fine: when cursor get line 75 and move down one line, jump to line 1 in MENU what is right.

Thanks

Originally Posted by zradu

Hello,
My problem is:
If cursor is in first line of OSD MENU and I want to move up, rather than jumping to line 75, cursor jump to line 61.
Thanks

I give up - this does not make any sense. Moving from first to last would require moving up one, and then moving 14 down...

You have to explain your goals in details. Read your first post once again, it asks for something completely different...

If you want help with optimising a loop, it would be helpful if you provide all information relevant to optimising that loop.

On a visual examination of your code, it appeared that cmd->curmenupos does not change, hence stahta01's suggestion. You have now revealed that it does change.

As such, you are wasting people's time. I suspect deliberately.