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.

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<UserIdentity> instead of map<string, int>.

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?

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<UserIdentity> instead of map<string, int>.

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 .

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.

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 .

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?

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 .

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.

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

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

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