Manage Ids so as to not exhaust them
sconklin opened this issue · 2 comments
Found during code review.
Seamly2D/src/libs/vpatterndb/vcontainer.cpp
Line 251 in cdcd529
As described in the following comment, IDs are not reused, and could be exhausted. Hitting this case would seem random and depend on all prior user actions, and be very difficult to reproduce. This should be fixed.
//TODO. Current count of ids are very big and allow us save time before someone will reach its max value.
//Better way, of cource, is to seek free ids inside the set of values and reuse them.
//But for now better to keep it as it is now.
There's nothing to fix if the concern is exhausing the number of (tool / node) ID's. The max number of ID's is defined by UINT_MAX in the c header limits.h - which is 4294967295.... 4.2+ Billion. There's no way in hell any user is ever going to hit the max ID in a pattern with that many tools. That many tools would surely overwhelm the CPU, and bog down well before that max ID is ever reached OR a user would have had to delete 4294 tools over a million times - an unlikely senario. I would have to ask... in 9+ years has a Seamly or Valentina user ever been presented with that critical error? Will they ever?
If on the other hand the goal is to not have gaps in the ID numbers, then it might be something to "fix", but I can say, the additional code to then track the use of ID's of when they are used, deleted and then free to use again is not worth the effort. Plus IMO it will only make the app run less efficient.
BTW... This is a pefect example of why over or bad commenting a codebase can be counter productive. IMO the fix is either to remove the comment altogther or write a valid comment as to why we might want TODO.
If someone decided to spend 24h/24 adding a node to his pattern every second without stopping and with a very powerful computer, it would take 136 years to hit the limit 😂😉