7

I have a structure which looks like

Person

ID        int
Name      nvarchar

Phone

ID        int
PersonId  int
Number    nvarchar
Make      nvarchar
Model     nvarchar
MultiSim  bit

One person can have many Phones

At the moment, the logic I have is when ever the user clicks the save button, the application receives a copy of the Person object (which includes an array Phone).

As mentioned the relationship is 1 to many. As such, on save, there is only 1 person and so it's easy to update the person (no need to delete and re-add).

Currently, I delete all of the Phone's associated with the person and re-add them.

The negative that I can see is:

  1. Additional time is taken as it has to delete and then re-add every Phone regardless of whether there was an amendment.
  2. The phone ID, which auto increments, gets bigger and bigger every time this process occurs

I can't find anything to explain if my current approach is wrong or not. Should I care that the ID is getting bigger? Should I be somehow checking the current Phone objects with the ones I want to save?

2
  • 1
    Does your code work even though the phone IDs are changing? Are you having performance problems related to the extra time it takes to do the "delete all then re-add" query approach? If not, then leave it alone and find another problem to solve! (And next time keep in mind that this pattern might not work for other data structures)
    – GHP
    Apr 29, 2019 at 13:27
  • Updated, but it was just an example @Machado, sadly your comment only pollutes the question Apr 30, 2019 at 10:36

4 Answers 4

6

I always use the delete all and re-add approach.

  1. Usually its quicker to just delete and re add than apply the logic that's required to update.

    This would probably require selects, locking while you run the checks etc.

  2. I use GUIDs for everything and thus avoid your id changing on insert problem.

    Although it has to be said that you could just change the insert to reuse the existing id if you have one.

  3. It's easy. Figure out an optimised approach when you hit a bottleneck.

  4. It is slower when you do a bulk insert, as it forces a loop.

    But this is easily avoided by deleting all the child objects with one statement (where personid in...) and then readding everything

3
  • This is usually a good approach. Sometimes, depending upon which table/entity I'm looking for, I add a trigger to store the previous data on a historic table, so we can track down the changes on that entity when necessary.
    – Machado
    Apr 29, 2019 at 13:30
  • Does not mean a unique ID is generated on each save?... If so it is not totally different to a new number as I am (although the id being incremented means no reuse where yours means any unique id) Apr 29, 2019 at 13:41
  • no you generate the id in code and keep it forever
    – Ewan
    Apr 29, 2019 at 14:40
2

If I had an issue with this approach, it wouldn't be the counter for the phone id, but rather the fact that it takes additional time. Though it may not be a big deal with a couple phone numbers (I can't imagine anybody having more than 10 in any realistic situation), keep in mind that if you're having to save all Person instances, now it is no longer a loop but a nested loop.

If you decide you want to delete and recreate Orders in a one to many relationship with Person, then that is yet again more work being done on save. It isn't very scalable and so you risk that one day your client is having wait times of 10 seconds or more for what would otherwise be a straightforward save to the database.

You ask why this shouldn't be a proper approach, but I reverse that question to you. Why wouldn't you simply keep track of which phone numbers have been added or removed and update accordingly? Is there a particular reason for doing so, or is the underlying reason to keep the code more simplistic? Since this may eventually be a problem in the long term, I would encourage you to consider simply adding and removing as necessary.

2

Additional time is taken as it has to delete and then re-add every Phone regardless of whether there was an amendment.

Making lots of changes where only one is required is inefficient and slow.
Making lots of changes where none is required is just silly (sorry).

The database has to log every change you make and, if you're deleting everything and re-adding it all, it's basically doing all of that twice, for no reason at all, if nothing's actually changed. And writing anything in a database is really, really slow compared to just reading it (updates always involve disk activity).
Also, if your database has any kind of "Data Auditing" going on (where changes are automatically transferred into "history" tables), then this approach will generate loads of "chaff" that anybody trying to investigate a real problem will have to "wade" through. (Been there, done that; not "Fun").

Ideally, you should never change the Primary Key (ID) of any record.
It should be allocated when the record is created and should remain constant until the record is finally destroyed. Imagine the chaos if your Bank "renumbered" everybodys' checking accounts when someone else closed theirs!

If there are any other tables that refer to your [Person_]Phone table and you're using Foreign Keys properly then you shouldn't be able to delete them at all!

Do the job properly; update any rows that the User changed, insert any that they added and delete any that they removed.

The phone ID, which auto increments, gets bigger and bigger every time this process occurs ... Should I care that the ID is getting bigger?

Probably not.
It depends on just how big it's getting. An int column will values up to about 2 Billion (2^31-1). Things will get a bit "messy" if you exceed that.
In this context, you're probably OK, but if you were to apply the same logic elsewhere, that might be a very different story.

0

This is an old question, but I am going to present an alternative opinion that this is definitely an antipattern. These are my concerns

  • Any other tables that point to the phone records will break when the primary key of your phone table changes. Typically this will be reports etc. In many cases a well designed database will refuse the delete as some foreign key is pointing to a record that you are attempting to delete.

  • You have limited future expansion. For instance. If a new phone has say 6 Sim cards, each with its own number then obviously you need to add a new table to support this. But imagine the messy extra code you need to add.

  • How do you handle concurrent but independent updates? For instance a slow updater changing the user password may span another update that was only changing a phone number? It looks like your approach would overwrite one update.

The very reason for this question is often that you are using an underpowered ORM that does just CRUD updates. I have always used the repository pattern, with targeted write operations that reflect the business needs (e.g. setPassword(userId, newPasswordHash), updatePhoneNumber(simCardId, newNumber)). Not only does this remove the concern of your question, it also removes the need for a lot of code that sorts out concurrent access problems.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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