# Ugly, can this be done easier?

This topic is 1050 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi all,

Just of curiosity, how would you achieve the same goal as I do with the code below?

It feels a bit 'clumsy', but I didn't find a way to do this easier/ less code repetition.

		D3DCOLOR selectedCol = D3DCOLOR_XRGB(255, 255, 0);
D3DCOLOR defCol = D3DCOLOR_XRGB(255, 255, 255);
D3DCOLOR tempCol;

{
{
// ITEMS
if(mGameStarted)
{
if(mCurrentMenuItem == 0) tempCol = selectedCol; else tempCol = defCol;
}

if(mCurrentMenuItem == 1) tempCol = selectedCol; else tempCol = defCol;
if(mCurrentMenuItem == 2) tempCol = selectedCol; else tempCol = defCol;
if(mCurrentMenuItem == 3) tempCol = selectedCol; else tempCol = defCol;
if(mCurrentMenuItem == 4) tempCol = selectedCol; else tempCol = defCol;

break;
}
}



Note; don't mind the switch has just one case for now, will be more later on

##### Share on other sites
You could get rid of the if(mCurrentMenuItem == x) stuff by using ternary operators like this:
mD3d.mD3dFont.PrintLarge("Some text", startMenuX, startMenuY + x * offsetMenuItem, mCurrentMenuItem == x ? selectedCol : defCol);
And you can also reduce the amount of code by adding all your string into a list and then iterate over it, so you only need the print code once.

##### Share on other sites

To demonstrate, something like

static const D3DCOLOR selectedCol = D3DCOLOR_XRGB(255, 255, 0);
static const D3DCOLOR defCol = D3DCOLOR_XRGB(255, 255, 255);

{
{
static const char* const items[]
= {
"Resume game",
"New game",
"Options",
"Quit game",
nullptr
};
uint idxItem = mGameStarted ? 0 : 1;
while (items[idxItem]!=nullptr) {
idxItem++;
}
} break;
}


Because I don't know what type the first parameter of PrintLarge actually expects, I've used (undesired) char* here for simplicity.

##### Share on other sites
Great, thanks.
Much cleaner this way, with both suggestions applied

##### Share on other sites

As a further enhancement you should consider making the menus data driven. e.g. the position and text of each entry should be within a configuration file which can be adjusted without a recompile. This usually makes for more readable and reusable code.

##### Share on other sites

As a further enhancement you should consider making the menus data driven. e.g. the position and text of each entry should be within a configuration file which can be adjusted without a recompile. This usually makes for more readable and reusable code.

{
// ..

public:
const char* ItemString (const int nID);
};

##### Share on other sites

thanks, will think about that too, might be useful especially if/ when I have more menu's

##### Share on other sites

Much better:

				for(size_t id=startFrom;id<mMainMenu.GetNrItems();++id)
{
}