1

I have a dilemma regarding my removing database function code.

Whenever I remove the database in vector with unique, I cannot think of writing the code that could fill the gap with the removed number (like I removed database ID3 and I want that the IDs of further databases would increment to have a stable sequence, so the database with ID4 would become ID3).

I also don't know how to decrement my static int counter.

File:

**void Database::Rem()
{
    int dddddet;
    
    cin >> dddddet;
    
    if (iter !=  DbMain.end())
    {
        DbMain.erase(iter);
    }
}**
std::istream &operator>>(std::istream &re, Base &product)
{
    
}

}
std::ostream &printnames(std::ostream &pr, Base &pro)
{
    pr << "\nID:" << pro.ID << "\nName:" << pro.name;
    return pr;
}

Header file:


"




  • um, why would you have a static int counter? – Marcus Müller May 27 '22 at 13:19
  • @MarcusMüller To help me with creating ID for each database –  May 27 '22 at 13:22
  • 1
    or to put it more precisely: I think your architecture needs a rework! The ID seems to be identical to the position of your Natobase inside the DbMain vector. So, don't store it inside the Natobase at all? As far as I can tell, you only need the ID to remove elements, and that's super awkward. Instead of saying "remove the Natobase with ID == 3", you could also simply say "dear vector remove the third Natobase". – Marcus Müller May 27 '22 at 13:26
  • when you want to know how many Natobases you've already created and are still there, simply `DbMain.size()`. – Marcus Müller May 27 '22 at 13:27
  • Far from a minimal example. all properties of the `Natobase` class except for the id (and perhaps one other property for purpose of demonstration) are irrelevant. Furthermore there's little point in returning an `int` by const reference. Simply returning as `int` is probably cheaper and avoids issues doesn't open up the possibility of a dangling reference... – fabian May 27 '22 at 13:32
  • The naming choice of `Natobase` does not seem appropriate in the current political context as it might make this question look like propaganda especially if associated to "remove". Please keep naming neutral (e.g. `Base`). – Christophe May 27 '22 at 14:39

2 Answers2

0

The thing you're doing here is called a design anti-pattern: Some structural idea that you could easily come up with in a lot of situations, but that's going to be a lot of trouble (i.e., bad). It's called a singleton: You're assuming there's only ever going to be one DbMain, so you store the length of that in a "global-alike" static member. Makes no sense! Simply use the length of DbMain. You should never need a global or a static member to count objects that you store centrally, anyways.

You never actually need the ID to be part of the object you store – its whole purpose is being the index within the dBMain storage. A vector is already ordered! So, instead of printing your .ID when you iterate through the vector, simply print the position in the vector. Instead of "find the base with ID == N and erase" you could do "erase the (DbMain.begin() + N) element" and be done with it.

Marcus Müller
  • 34,677
  • 4
  • 53
  • 94
0

The problem in your design is that you seem somehow to associate the unique ID with the index in the vector, as you initialize the IDs with a static counter. This is not a good idea, since:

  • you would need to renumber a lot of IDs at each delete and this would make it very difficult to maintain cross-references.
  • you could have several databases each with its own set of ids.
  • there is a risk that you'd not get the counter to a proper value, if you add bases without reading them.

Moreover, iterating sequentially looking for an ID is not very efficient. A more efficient approach would be to store your bases in a std::map: this allows to find IDs very efficiently, indexing by ID instead of sequential number.

Your only concern would then be to ensure uniqueness of IDs. Of course, your counter approach could work, if you make sure that it is updated whenever a new base is created, and that the state is persisted with the database in the text file in which you'll save all this. You just have to make clear that the IDs are not guaranteed to be sequential. A pragmatic way to ensure contribute to this understanding is to issue an error message if there is an attempt to delete a record that was not found.

Christophe
  • 68,716
  • 7
  • 72
  • 138