Skip to main content
gamedev.net
🔒 Locked

c++ use map with struct

Started by randydom Oct 10, 2018 at 10:49 AM 17 replies 3.8k views
Original Post
randydom
randydom

Hello every one , please i want to be sure this's the best + optimized way , or even the worst way ?

what i want is to be able to export a struct from my DLL .


struct UserIdentity
{
std::string name;
int id;
};

std::map<std::string, int> m_UserIdentities = { { "Bob", 100 }, { "Jone", 101 },
            { "Alice", 102 }, { "Doe", 103 } };
            
/*
* Will be used in a DLL that will export UserIdentity struct
* OUT _UserIdentity
*/
void Ui_export(UserIdentity *_UserIdentity)
{
for (auto& t : m_UserIdentities)
    {
     _UserIdentity->name = t.first;
     _UserIdentity->id   = t.second;
    }
        
}

Regards.

pcmaster
pcmaster

In your function (which lacks a name), you're overwriting your output variable _UserIdentity all over again.

Also, your m_UserIdentities could be directly std::map instead of map.

Furthermore, the "m_" prefix usually signifies "members" of classes. In your example, m_UserIdentities looks like a global variable, it'd name it g_UserIdentities, if you prefer this convention.

Does https://stackoverflow.com/questions/6840576/how-to-export-a-c-class-from-a-dll help?

randydom
randydom
3 minutes ago, pcmaster said:

In your function (which lacks a name), you're overwriting your output variable _UserIdentity all over again.

Also, your m_UserIdentities could be directly std::map instead of map.

Furthermore, the "m_" prefix usually signifies "members" of classes. In your example, m_UserIdentities looks like a global variable, it'd name it g_UserIdentities, if you prefer this convention.

Does https://stackoverflow.com/questions/6840576/how-to-export-a-c-class-from-a-dll help?

@ pcmaster So many thanks , can you please help me to fix or optimize this :


void Ui_export(UserIdentity *_UserIdentity)
{
for (auto& t : m_UserIdentities)
    {
     _UserIdentity->name = t.first;
     _UserIdentity->id   = t.second;
    }
        
}

PS : I fixed the function name .


Zakwayda
Zakwayda

DLL issues aside, as pcmaster pointed out, you're writing to the output parameter repeatedly, which seems to be conceptually wrong.

Perhaps you could clarify what it is you want (or expect) the function to do.

randydom
randydom

Thank you , let me explain what i'm trying to achieve :

I've a class ( TIdenetyClass ) contains users information ( Name , ID ... etc ) , it's mapped as follows :


std::map<int, std::shared_ptr<TIdenetyClass >> _IdenetyClass;

so i fill my


TIdenetyClass

and insert it in the map ; here every things go well .

then i want the DLL consumer to be able to get all the users stored in my


TIdenetyClass class

, But here i don't want to export the class itself but a struct , so each time the consumer want to get all users it will invoke that by


/*
* Get all users and export them in the UserIdentity struct
*/
void get_all_users(UserIdentity *_UserIdentity)
{
  // loop and fill my UserIdentity's fields 
  // this will get all current users 
  std::lock_guard<std::mutex> locker(_locker);
for (auto& t : _IdenetyClass)
    {
     _UserIdentity->id   = t.second.id;
     _UserIdentity->name  = t.second.name;
    }
        
}

That's what i'm trying to achieve , the part i'm trying to be sure is correct and optimized is the function


void get_all_users

thank you .

Zakwayda
Zakwayda

I'll just ask a couple more questions to try to clarity things further.

- What does the name 'TIdenetyClass' mean? Is it a 'T' prefix plus the word 'Idenety'? If so, what does 'T' mean, and what does 'Idenety' mean? (Note that I'm not questioning your naming conventions or anything. I'm just trying to better understand what the names mean.)

- I still don't understand the implementation of get_all_users(). An instance of UserIdentity only stores a single name-ID pair. How are you expecting to store multiple such pairs in a single instance?

randydom
randydom
11 minutes ago, Zakwayda said:

I'll just ask a couple more questions to try to clarity things further.

- What does the name 'TIdenetyClass' mean? Is it a 'T' prefix plus the word 'Idenety'? If so, what does 'T' mean, and what does 'Idenety' mean? (Note that I'm not questioning your naming conventions or anything. I'm just trying to better understand what the names mean.)

- I still don't understand the implementation of get_all_users(). An instance of UserIdentity only stores a single name-ID pair. How are you expecting to store multiple such pairs in a single instance?

1 : It's just a naming convention ( i'm a delphi programmer and i'm just following the delphi classes naming convention ) that's all .

2 : I'm looping to get all Users's information and store them in the UserIdentity ; something like creating an array of UserIdentity

I thought about this :


void get_all_users(UserIdentity **_UserIdentity)
{
 // But here i really didnt' know how to 
 // impelent it like with *_UserIdentity
}

if this implementation is incorrect you're so welcome to fix it and i'll be so thankful .


Zakwayda
Zakwayda
Quote

It's just a naming convention ( i'm a delphi programmer and i'm just following the delphi classes naming convention ) that's all .

I'm still curious as to what 'Idenety' means, but perhaps it's not important.

Quote

I'm looping to get all Users's information and store them in the UserIdentity ; something like creating an array of UserIdentity

I haven't done much with Windows in a while, so I'm not going to comment on the DLL aspect of things. There may of course be additional considerations related to that.

Your original implementation doesn't do what you want (as you probably know by now). All it does (as pcmaster pointed out) is write the name-ID pairs to the same UserIdentity instance repeatedly. If the map is empty, the UserIdentity instance will be unaltered. If the map contains one entry, you may get the desired results (more or less), but only circumstantially. If the map has more than one entry, only the last one will be recorded.

'UserIdentity *' could also be treated as an array. You're not treating it as one though, and you'd need knowledge of its size to write to it safely. 'UserIdentity **_UserIdentity' might be appropriate if you wanted the function to be responsible for creating the array. Typically though, in that case you'd just create the array locally and return it, I would think (although in both cases you'd probably need to convey the size of the array to the user somehow). Lastly, barring other considerations (again, I'm not addressing the DLL issue here), it's common to use RAII containers such as std::vector for things like this to alleviate the burden of manual memory management (and to avoid the kinds of bugs that can come with it).

I think the most straightforward implementation of a function like this would create a std::vector instance locally, populate it, and return it. Again though, perhaps there are other considerations at play.

Edit: Minor correction.

randydom
randydom

@Zakwayda so many thanks , please can you help in implementing this :

std::vector instance locally or the 'UserIdentity **_UserIdentity' , i really appreciate that .

Zipster
Zipster

The best approach really depends on who you expect to be consuming this data. You have to be really careful when using standard library types such as std::string across binary boundaries, or anything that has a layout that can change between platforms and/or build environments. Even if you take all the necessary steps to ensure these are identical, only C++ code knows about std::string. Perhaps this isn't an issue if all your software components are written in C++, which is why so much of this depends on your intent :)

randydom
randydom

Ok i tried to re-implement my function so that it exports an array of _UserIdentity in this way :


void get_all_users(UserIdentity **_UserIdentity)
{
  std::lock_guard<std::mutex> locker(_locker);
 for (auto& t : _IdenetyClass)
    {
     (*_UserIdentity)->id   = t.second.id;
     (*_UserIdentity)->name  = t.second.name;
    }
}

So is my new implementation correct please ?


Zakwayda
Zakwayda

Like Zipster said, there may be other considerations at play here.

As far as the code itself goes though, a couple questions. First, does it compile? Second, have you tested it, and does it produce the desired results?

Something you might consider is setting aside the DLL issue for now and just focusing on implementing this in isolation, maybe even in a self-contained single-file terminal or command-line app. That way you could post your code in its entirety for others to look at.

What you have there appears to be the same thing you had before, just expressed differently. As such, it still likely doesn't do what you want.

Although I mentioned std::vector earlier, you might benefit from implementing this without such tools first in order to better understand the underlying concepts. How familiar are you with topics such as arrays, pointers, and manual memory management?

randydom
randydom

1- yes it does and it prints the result in the debug window

To be honest with you i'm 5/10 with topics such as arrays , pointers and manual memory management .☹️

Zakwayda
Zakwayda
17 minutes ago, randydom said:

1- yes it does and it prints the result in the debug window

To be honest with you i'm 5/10 with topics such as arrays , pointers and manual memory management .☹️

I see. Regarding the testing, if it seems like it's working, I'm wondering if it's just circumstantial (e.g. maybe you're just testing it with one user identity). If you're not sure about that, you could post your test code for us to take a look at.

As for the topics under discussion (arrays, etc.), I think getting more comfortable with those would be helpful. The function you're trying to implement is, I think, a reasonable test bed for that, as it touches on a number of relevant issues, such as pointers, arrays, memory management, and ownership semantics. Although eventually using something like std::vector would probably be preferable, implementing it 'manually' might be a good starting point.

Fulcrum.013
Fulcrum.013
1 hour ago, Zakwayda said:

I'm still curious as to what 'Idenety' means, but perhaps it's not important.

For name convention of Borland's made libraries typename identifiers began from "T" character to easily separate types and instances identifiers by name.

12 hours ago, randydom said:

what i want is to be able to export a struct from my DLL .

It is wrong idea to use a templates (especially stl) and any dynamic created object to pass it to DLL. It no any warranty that it have a compatible memory managers and compiled using same implementation of STL or other templates (that can have incompatible formats of underlaying data structures under hood).


2 hours ago, randydom said:

i'm a delphi programmer

Also you trying to follow a delphi way of dlls interoperation. Delphi/C++ Builder have especily made for dlls interoperation technology implemented by FastMM memory manager that uses especialy dll to serve ineroperability of objects that contain dynamic buffers under hood (strings dynarras and so on). But it interoperability applicable to Delphi/C++ Builder compiled dlls only (and most likely by same version only). In common case allocating a object in context of one dll and deleting it in context of another just will corrupt heap and lead to crash.

#define if(a) if((a) && rand()%100)
randydom
randydom

@ Fulcrum.013 many thanks so what do you suggest as an alternative way for achieving the export procedure ?

Zakwayda
Zakwayda
7 minutes ago, Fulcrum.013 said:

For name convention of Borland's made libraries typename identifiers began from "T" character to easily separate types and instances identifiers by name.

I was asking about the word 'Idenety', not the prefix.

Fulcrum.013
Fulcrum.013
25 minutes ago, randydom said:

so what do you suggest as an alternative way for achieving the export procedure

Really it only 3 ways exists - use a raw data and functions only to interdll calls like low-level APIs do, use a COM that have warranty that delete from same dll that initiyally create objec wil be used (by refcounting), or reinvent something similar to FastMM like .Net do (really it made by Borlands concepts enlarged to system-wide scale, but downgraded by restrictions of GC flexibility).

Other way is to review architecture and avoid using DLLs in similar cases. Really any templates intendend to inline code that have to be very fast, while dll calls have a huge overhead (code from it can not be inlined and only indirect call can be performed) and intended to increase flexibility, so better way is to avoid interdll calls anywhere where it possible.

#define if(a) if((a) && rand()%100)

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.