-2
\$\begingroup\$

My project has many different objects from different classes, and I want each object to have a unique ID. IDs should be unique within each class type, but can be the same among different classes. For example Foo object1 and Foo object2 must have different unique IDs, but Foo object1 can have the same ID number as Bar object1.

Here is what I thought of doing, and I would appreciate feedback on whether this is a good solution, or whether I should try to implement this through a class instead of a function.

template <typename T>
void giveID(T& obj) { static int id = 1; obj.id = id++; }

struct Foo
{
    Foo() { giveID(*this); };
    int id;
};

struct Bar
{
    Bar() { giveID(*this); };
    int id;
};

int main()
{
    Foo foo1; // ID = 1
    Foo foo2; // ID = 2
    Bar bar1; // ID = 1
    Bar bar2; // ID = 2
}

Also, let's say an object gets ID number 5, and then the object is deleted, the ID counter that assigns IDs might be up to ID 10. ID number 5 is unused. Is it a good idea to assign IDs that were once used but are no longer? In other words would it be a good idea to search through available unused ID numbers and assign that, or to simply keep the counter incrementing and having certain ID numbers unassigned?

Thanks for any feedback.

Edit: I've come up with another way using a templated class. Note, static inline class members are a C++17 feature:

template <typename T>
struct giveID
{
    giveID(T& obj) { obj.id = id++; }
    static inline int id = 1;

};

struct Foo 
{ 
    Foo() { giveID<Foo>(*this); }
    int id; 
};

struct Bar
{
    Bar() { giveID<Bar>(*this); }
    int id;
};

int main()
{
    Foo foo1; // ID = 1
    Foo foo2; // ID = 2
    Bar bar1; // ID = 1
    Bar bar2; // ID = 2

    return 0;
}
\$\endgroup\$

closed as off-topic by hoffmale, Incomputable, yuri, vnp, t3chb0t Aug 12 '18 at 16:42

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – hoffmale, Incomputable, yuri, vnp, t3chb0t
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ @Incomputable The second version seems to compile fine. Sorry, which return type is missing? The second version just creates a temporary and assigns the ID through the temporary's constructor, I'm pretty sure. \$\endgroup\$ – Zebrafish Aug 12 '18 at 14:34
  • \$\begingroup\$ I apologize for being too hasty. I confused it with the function name. Yet, the second point still stands. I'm not the downvoter, but I did vote to close the post. You could just add some code to create objects, some manipulations of them that are actually relevant to the project. No need to copy the whole project, but you can. The only necessity is for the code to be relevant to the project. \$\endgroup\$ – Incomputable Aug 12 '18 at 14:35
  • \$\begingroup\$ @Incomputable Sorry, this is my first post on here. I thought I could get an opinion on whether this is an acceptable or terrible idea. But as I understand it my code needs to show more context about my usage. \$\endgroup\$ – Zebrafish Aug 12 '18 at 14:38
  • \$\begingroup\$ It compiles fine only when using C++17 or higher. Right now this is not made clear by you. So people would consider this non-working. \$\endgroup\$ – yuri Aug 12 '18 at 14:40
  • \$\begingroup\$ @yuri Yeah that's true. If you're talking about the static inline initialisation, it's only a C++17 onwards feature. It's a feature I like very much. \$\endgroup\$ – Zebrafish Aug 12 '18 at 14:42
1
\$\begingroup\$

The first question: yes, acceptable (putting aside questions like should an architecture that requires unique ids be restructured—there's no such thing as universal solution after all.) Note that it probably makes sense to return an id rather than assign it by reference, as the latter allows to assign ids in init-list (and you can declare id const, if you don't need to assign objects.)

The second question: while quite possible, are you sure you really need to keep track of all the allocated/released ids? resource bookkeeping with static data storages (and maybe even garbage collection), all that stuff that immediately becomes necessary, eh?

Well, if you really need to, instead of giveID you'll need a whole static objects that assigns ids, accepts freed ones (this method is called from object's destructor) etc.

template<class T> class IdSource {
    std::size_t frontier = 0;
    std::stack<std::size_t> released;
public:
    std::size_t assign() {
        if(released.empty()) return ++frontier;
        int retVal(released.top());
        return released.pop(), retVal;
    }
    void release(std::size_t id) { released.push(id); }
};

template<typename T> inline IdSource<T> &idSource() {
    static IdSource<T> retVal;
    return retVal;
};

struct Foo {
    Foo(): id(idSource<Foo>().assign()) {}
    ~Foo() { idSource<Foo>().release(id); }
private:
    std::size_t id;
};

And note that additional something is needed to make it thread-aware.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot, I like this. I noticed the ID started at 0, and when you gave it out wrote ++frontier. Is this the same basically as how I had it? Which is starting the ID at 1 and giving the ID as frontier++ ? \$\endgroup\$ – Zebrafish Aug 12 '18 at 14:58
  • \$\begingroup\$ Yes, it's preincrement. \$\endgroup\$ – bipll Aug 12 '18 at 15:46
  • \$\begingroup\$ But please note this way you'll have to carefully implement copying/movement, to not double release ids or allow them to leak. \$\endgroup\$ – bipll Aug 12 '18 at 17:51

Not the answer you're looking for? Browse other questions tagged or ask your own question.