questions about data strcuture

[John Doe]

Hi,

I am currently developping a software where items that will be inserted
into a graphical widget ListCtrl are first defined in static array as
shown below :

enum TUiContext
{
EUiContextMain = 0,
EUiContextSchedule,
EUiContextSettings,
EUiContextSubscription,
EUiContextCount
};

typedef struct
{
int TextId;
int ImgId;
int Param;
int nOptInfo;
TCHAR szImgName[MAX_PATH];
} ListInfo_t, *LPListInfo_t;


ListInfo_t CMainView::ms_listInfo_Main[]=
{
// String ID, Img, Param = CmdBarId Enabled ImgName
{ IDS_MENU_BACKUP, 0, IDM_MENU_CMDBAR_BACKUP, TRUE, _T( "" ) },
{ IDS_MENU_SCHEDULE, 2, IDM_MENU_CMDBAR_OPTIONS, TRUE, _T( "" ) },
{ IDS_MENU_RESTORE, 1, IDM_MENU_CMDBAR_RESTORE, TRUE, _T( "" ) },
{ IDS_MENU_MANAGE_SUBSCRIPTION, -1, IDM_MENU_MANAGE_SUBSCRIPTION,
TRUE, _T( "Menu_Account_Manage.png" ) },
};

ListInfo_t CMainView::ms_listInfptions[]=
{
// String ID, Img, Param = CmdBarId
{ IDS_MENU_FREQUENCY, 0, IDM_MENU_SCHEDULER, TRUE, _T( "" ) },
{ IDS_MENU_CONTENT, 1, IDM_MENU_SELECTDB, TRUE, _T( "" ) },
};
....


Actually in functions of some parameters, some items won't be inserted
and the field nOptInfo is used for this purpose. If this field equals 1
it will be inserted into the List.

So I have a method called InitResources that check the config parameters
and update nOptInfo for each array.
Once this has been done, I build a vector as shown below :

void CMainView::InitResources()
{
std::vector<ListInfo_t> vecListInfo;
std::map<TUiContext, std::vector<ListInfo_t> > listMap;

// Test config parameters and update nOptInfo
...

//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
// Now build vector from ms_listInfo_Main
//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
vecListInfo.clear()
for (int i = 0; i < _countof(ms_listInfo_Main); i++)
{
if (ms_listInfo_Main.nOptInfo == TRUE){
vecListInfo.push_back(ms_listInfo_Main);
}
}
listMap[ EUiContextMain ] = vecListInfo;

//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
// Now build vector from ms_listInfptions
//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
vecListInfo.clear()
for (int i = 0; i < _countof(ms_listInfptions); i++)
{
if (ms_listInfptions.nOptInfo == TRUE){
vecListInfo.push_back(ms_listInfptions);
}
}
listMap[ EUiContextSchedule] = vecListInfo;
...

}

I find all this code very ugly and I would like to suggestion to improve
it. I am doing all this because I am switching between different
graphical context and before to do it I save the index of current
selected item in my ListCtrl.

 

[Jerry Coffin]

typedef struct
{
int TextId;
int ImgId;
int Param;
int nOptInfo;
TCHAR szImgName[MAX_PATH];
} ListInfo_t, *LPListInfo_t;

Since you're apparently using nOptInfo as a boolean, I'd advise defining
it as a boolean. Although it's unrelated to any of the questions at
hand, I'd look carefully at whether szImgName couldn't be an std::string
as well.

vecListInfo.clear()

Rather than defining well ahead of time and clearing when used, I'd
define this immediately before use, in which case it'll definitely be
empty.

for (int i = 0; i < _countof(ms_listInfo_Main); i++)
{
if (ms_listInfo_Main.nOptInfo == TRUE){
vecListInfo.push_back(ms_listInfo_Main);
}
}

Then I'd replace this with std::remove_copy_if:

std::remove_copy_if(ms_listInfo_Main, end(ms_listInfo_Main), isOpt());

where isOpt looks something like:

struct isOpt {
bool operator()(ListInfo_t const &li) { return !li.nOptInfo; }
};

At least if I understand your code correctly, I'd then create a single
vector containing (pointers to?) the data you currently have in your
ms_ListInfo_*. I'd then turn the bit of code above (the part that uses
std::remove_copy_if) into a small function (which, for now, I'll call
build_list) that could be called in a loop:

for (int i=0; i<EUiContextCount; i++)
listMap = build_list(i, ms_ListInfo);

then I'd note that this is really equivalent to std::transform:

std::transform(ms_ListInfo.begin(), ms_ListInfo.end(),
listMap, build_list());

 

[Pascal J. Bourguignon]

Hi,

I am currently developping a software where items that will be
inserted into a graphical widget ListCtrl are first defined in static
array as shown below :

enum TUiContext
{
EUiContextMain = 0,
EUiContextSchedule,
EUiContextSettings,
EUiContextSubscription,
EUiContextCount
};

typedef struct
{
int TextId;
int ImgId;
int Param;
int nOptInfo;
TCHAR szImgName[MAX_PATH];
} ListInfo_t, *LPListInfo_t;


ListInfo_t CMainView::ms_listInfo_Main[]=
{
// String ID, Img, Param = CmdBarId Enabled ImgName
{ IDS_MENU_BACKUP, 0, IDM_MENU_CMDBAR_BACKUP, TRUE, _T( "" ) },
{ IDS_MENU_SCHEDULE, 2, IDM_MENU_CMDBAR_OPTIONS, TRUE, _T( "" ) },
{ IDS_MENU_RESTORE, 1, IDM_MENU_CMDBAR_RESTORE, TRUE, _T( "" ) },
{ IDS_MENU_MANAGE_SUBSCRIPTION, -1,
IDM_MENU_MANAGE_SUBSCRIPTION, TRUE, _T( "Menu_Account_Manage.png"
) },
};

ListInfo_t CMainView::ms_listInfptions[]=
{
// String ID, Img, Param = CmdBarId
{ IDS_MENU_FREQUENCY, 0, IDM_MENU_SCHEDULER, TRUE, _T( "" ) },
{ IDS_MENU_CONTENT, 1, IDM_MENU_SELECTDB, TRUE, _T( "" ) },
};
...


Actually in functions of some parameters, some items won't be inserted
and the field nOptInfo is used for this purpose. If this field equals
1 it will be inserted into the List.

So I have a method called InitResources that check the config
parameters and update nOptInfo for each array.
Once this has been done, I build a vector as shown below :

void CMainView::InitResources()
{
std::vector<ListInfo_t> vecListInfo;
std::map<TUiContext, std::vector<ListInfo_t> > listMap;

// Test config parameters and update nOptInfo
...

//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
// Now build vector from ms_listInfo_Main
//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
vecListInfo.clear()
for (int i = 0; i < _countof(ms_listInfo_Main); i++)
{
if (ms_listInfo_Main.nOptInfo == TRUE){
vecListInfo.push_back(ms_listInfo_Main);
}
}
listMap[ EUiContextMain ] = vecListInfo;

//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
// Now build vector from ms_listInfptions
//$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$
vecListInfo.clear()
for (int i = 0; i < _countof(ms_listInfptions); i++)
{
if (ms_listInfptions.nOptInfo == TRUE){
vecListInfo.push_back(ms_listInfptions);
}
}
listMap[ EUiContextSchedule] = vecListInfo;
...

}

I find all this code very ugly

Yes, it is.

and I would like to suggestion to
improve it. I am doing all this because I am switching between
different graphical context and before to do it I save the index of
current selected item in my ListCtrl.

You need more abstraction. Define classes to hold your data, instead
of storing it in POD (plain old data) structures. When you build
these objects, they will be able to implement any consistency check
you need, so you won't have to check the POD and you won't have to
convert.


With smart use of operators, such as operator<< or operator, you can
make it nice enough. Optional attributes can be fed optionnaly, after
the construction.

Since you seem to have here a hierarchical structure of contexts, be
sure to make them have reference semantics, not value semantics,
otherwise putting them in a std container would project them and lose
data.



Instead of using #define for constants or enums, you should use true
const or enum declarations, with the help of namespaces to have nice
names and avoid collisions:

namespace ids {
namespace menu {
enum menu { backup, schedule, restore, manage_subscription }; }}

Notice the duplication of namespace X { enum X which allows to use
the namespace to qualify the enum constants, without having to
reinvent the wheel by prefixing the constants with the enum name to
avoid collisions.



void CMainView::InitResources()
{
MainContext mctxt;
mctxt<<(MInfo(ids::menu::backup, idm::menu::cmdbar::backup) <<Image(0))
<<(MInfo(ids::menu::schedule,idm::menu::cmdbar:ptions) <<Image(2))
<<(MInfo(ids::menu::restore, idm::menu::cmdbar::restore) <<Image(1))
<<(MInfo(ids::menu::manage_subscription,idm::menu::manage::subscription)
<<ImageName ("Menu_Account_Manage.png"))
<<(MInfo(ids::menu::wild, idm::menu::cmdbar::broken) <<Disabled()
<<Color(ids::color::red)
<<Etc("you can define whatever attribute you want"));

ScheduleContext sctxt;
sctxt<<(SInfo(ids::menu::frequency,idm::menu::scheduler) <<Image (0))
<<(SInfo(ids::menu::content, idm::menu::selectdb) <<Image (1));

std::map<TUiContext,Context> listMap;

// Test config parameters and update nOptInfo
// ...
listMap[EUiContextMain ]=nctxt;
listMap[EUiContextSchedule]=sctxt;
// ...
}